-
Notifications
You must be signed in to change notification settings - Fork 7
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
Test suite: Only run the c1,c2
hostname test in CI
#29
base: master
Are you sure you want to change the base?
Test suite: Only run the c1,c2
hostname test in CI
#29
Conversation
b069bdb
to
7e88517
Compare
3fcb9fe
to
7c8d28c
Compare
Also, add some debug printing to help when running the tests on a local cluster. Co-authored-by: jbphyswx <jbenjami@caltech.edu>
7c8d28c
to
b7f09cb
Compare
Alright, @kleinhenz this is now ready for review. @jbphyswx This should make it easier for people to run the |
test/script.jl
Outdated
if hosts != ["c1", "c1", "c2", "c2"] | ||
msg = "Test failed: observed_hosts=$(hosts) does not match expected_hosts=[c1, c1, c2, c2]" | ||
error(msg) | ||
const is_ci = parse(Bool, strip(get(ENV, "CI", "false"))) |
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 this actually present in our CI since we are running inside the docker compose cluster?
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.
Ah, that's a really good point. No, I don't think it is.
Hmmm. I'll have to figure something else out.
Depends on:
@assert
s to errors, and change bareusing Foo
tousing Foo: name, anothername, ...
#28