Skip to content
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

Merged
merged 2 commits into from
Jan 30, 2025

Conversation

DilumAluthge
Copy link
Member

@DilumAluthge DilumAluthge commented Dec 31, 2024

@DilumAluthge
Copy link
Member Author

This depends on #28. So #28 should be merged first, and then this PR can be merged.

@DilumAluthge DilumAluthge marked this pull request as ready for review December 31, 2024 06:12
@DilumAluthge DilumAluthge force-pushed the dpa/ci-hostnames branch 2 times, most recently from 3fcb9fe to 7c8d28c Compare January 2, 2025 00:15
@DilumAluthge
Copy link
Member Author

Alright, @kleinhenz this is now ready for review.

@jbphyswx This should make it easier for people to run theSlurmClusterManager.jl package tests on their own local Slurm clusters.

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")))
Copy link
Collaborator

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?

Copy link
Member Author

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I have a different approach now.

If the environment variable is not set (e.g. if you are inside of our Docker Compose cluster), we default to is_ci = true.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So if you look at the CI logs on this PR (e.g. this CI log), you'll see the following message:

[ Info: This is CI, so we will perform the hostname test

This indicates that the test is being run on our CI, as desired.

@DilumAluthge DilumAluthge marked this pull request as draft January 17, 2025 03:37
@DilumAluthge DilumAluthge force-pushed the dpa/ci-hostnames branch 2 times, most recently from 021e827 to 21e033c Compare January 20, 2025 21:13
@DilumAluthge DilumAluthge marked this pull request as ready for review January 21, 2025 00:04
@kleinhenz kleinhenz merged commit 0703f19 into JuliaParallel:master Jan 30, 2025
4 checks passed
@DilumAluthge DilumAluthge deleted the dpa/ci-hostnames branch January 30, 2025 03:08
@DilumAluthge DilumAluthge restored the dpa/ci-hostnames branch January 30, 2025 03:08
@DilumAluthge DilumAluthge deleted the dpa/ci-hostnames branch January 30, 2025 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants