-
Notifications
You must be signed in to change notification settings - Fork 41
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
testsuite: add test for double-booking #1044
Conversation
The fedora34 builder apparently did not reproduce the issue - I'm guessing maybe the timing is a little tight:
|
Well the 2s epilog I thought I had included was actually not enabled - fixed that. |
Problem: alloc-check test was simplified when it was submitted as flux-framework/flux-sched#1044. Simplify script in a similar way. For sched-simple, reduce the number of jobs submitted from 5 to 3 to expedite testing.
Problem: alloc-check test was simplified when it was submitted as flux-framework/flux-sched#1044. Simplify script in a similar way. For sched-simple, reduce the number of jobs submitted from 5 to 3 to expedite testing.
Problem: there is no test script for ensuring fluxion never allocates the same resources to multiple jobs. Add a sharness script that utilizes the alloc-check plugin to account for allocated resources and catch errors. At this point, it just includes an "expected failure" test for flux-framework#1043.
Test commits squashed, and a "skip" added for older flux-core as discused in ☕ time today. |
Codecov Report
@@ Coverage Diff @@
## master #1044 +/- ##
======================================
Coverage 74.3% 74.4%
======================================
Files 86 86
Lines 9432 9432
======================================
+ Hits 7017 7020 +3
+ Misses 2415 2412 -3 |
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.
Tested this and LGTM. The exit: Invalid number:
error seems to be a sharness bug (exit
is being called with an empty arg, i.e. exit ""
), though why it only manifests here is a mystery to me. Doesn't seem too important for now.
Thanks! I'll set MWP. |
Problem: flux-framework#1044 introduced tests to check for allocation failures due to jobs that exceed their time limit. PR flux-framework#1044 had to define the two `alloc-check` tests as `test_expect_failure` because the mechanism to prevent multiple booking was not in place. Update the two `alloc-check` tests to test_expect_success since the PR of this commit fixes multiple booking in Fluxion.
Problem: flux-framework#1044 introduced tests to check for allocation failures due to jobs that exceed their time limit. PR flux-framework#1044 had to define the two `alloc-check` tests as `test_expect_failure` because the mechanism to prevent multiple booking was not in place. Update the two `alloc-check` tests to test_expect_success since the PR of this commit fixes multiple booking in Fluxion.
Problem: there is no test script for ensuring fluxion never allocates the same resources to multiple jobs.
Add a sharness script that utilizes the alloc-check plugin to account for allocated resources and catch errors. At this point, it just includes an "expected failure" test for #1043.
Note that this PR requires a version of flux-core with flux-framework/flux-core#5304 (just merged). I wanted to get this posted as I saw that @milroy was looking into #1043.