-
Notifications
You must be signed in to change notification settings - Fork 71
Modify tests directory structure #83
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
b1a97b8
to
1e6c9ea
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌
|
||
##################### | ||
# Integration tests # | ||
##################### | ||
RUST_BACKTRACE=1 RUST_LOG=info cargo run & | ||
RUST_BACKTRACE=1 cargo run $FEATURES \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there no need to sleep
after this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, because we do a cargo build
before so that is alsmost instantaneous 😸
tests/cross_compilation/config.toml
Outdated
listener_type = "DomainSocket" | ||
timeout = 200 # in milliseconds | ||
|
||
# No providers are compiled in for the cross-compilation example. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure this is the case - the providers are compiled, this config file is just for actually running the service, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well as of now, the cross-compilation test only compiles the service for Aarch64 and does not even execute any test, so this file is useless. I will remove it.
RUN apt-get update && \ | ||
apt-get install -y wget automake autoconf libtool pkg-config && \ | ||
apt-get install -y curl libssl-dev | ||
# These libraries are needed for bindgen as it uses libclang.so |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
? Needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will remove that
# It is meant to be executed inside one of the container which Dockerfiles | ||
# are in tests/per_provider/provider_cfg/*/. | ||
# | ||
# Usage: ./tests/per_provider/ci.sh PROVIDER_NAME |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be worth adding a check at the beginning of scripts that require an argument and print something like this if that argument wasn't provided
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea! Will add.
In order to execute the tests under different single providers, under all providers and for cross-compilation, split the tests directory into sub-directories. Each one contains a specific configuration file to run the tests under. Please check issue parallaxsecond#69 for details. Signed-off-by: Hugues de Valon <[email protected]>
1e6c9ea
to
8212cb8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!!
In order to execute the tests under different single providers, under
all providers and for cross-compilation, split the tests directory into
sub-directories. Each one contains a specific configuration file to run
the tests under.
Please check issue #69 for details.
Signed-off-by: Hugues de Valon [email protected]