flux-core icon indicating copy to clipboard operation
flux-core copied to clipboard

[WIP] testsuite: add timeout job tests

Open chu11 opened this issue 2 years ago • 3 comments

So I'm admittedly not sure if the pros outweigh the cons on this. This was initially going to be just comments to point readers to the tests in t2900-job-limits, but as I was skimming things I realized "oh this isn't covered ... and this isn't covered ... and ..." and I slowly started adding things.

The addition of the timed out job to the job list tests is probably racy, which we can add things to try and limit that.

I maybe added too many permutations to the job list filtering tests, which can be whittled down.

Do the pros outweigh the cons?

chu11 avatar Aug 16 '22 23:08 chu11

The added tests seem pretty reliable actually, and it is good that we add tests for querying timeout jobs. I'd say since you've done the work here already, let's go ahead and merge this. Thanks!

grondo avatar Aug 17 '22 01:08 grondo

Suggestion: s/timed out/timeout/ in PR title since that more closely matches the literal status string being tested.

grondo avatar Aug 17 '22 01:08 grondo

Codecov Report

Merging #4501 (b8c3da9) into master (ef1cb64) will decrease coverage by 0.00%. The diff coverage is n/a.

:exclamation: Current head b8c3da9 differs from pull request most recent head 8365c74. Consider uploading reports for the commit 8365c74 to get more accurate results

@@            Coverage Diff             @@
##           master    #4501      +/-   ##
==========================================
- Coverage   83.38%   83.37%   -0.01%     
==========================================
  Files         401      401              
  Lines       67549    67539      -10     
==========================================
- Hits        56324    56314      -10     
  Misses      11225    11225              
Impacted Files Coverage Δ
src/common/libsubprocess/subprocess.c 87.89% <0.00%> (-0.30%) :arrow_down:
src/common/libsdprocess/sdprocess.c 68.88% <0.00%> (-0.26%) :arrow_down:
src/cmd/flux-job.c 87.29% <0.00%> (-0.22%) :arrow_down:
src/shell/output.c 76.54% <0.00%> (-0.16%) :arrow_down:
src/common/libterminus/terminus.c 86.06% <0.00%> (+0.24%) :arrow_up:
src/modules/job-info/guest_watch.c 77.02% <0.00%> (+0.27%) :arrow_up:
src/common/libsubprocess/server.c 61.35% <0.00%> (+0.81%) :arrow_up:
src/cmd/flux-jobs.py 97.17% <0.00%> (+1.12%) :arrow_up:

codecov[bot] avatar Aug 17 '22 22:08 codecov[bot]

re-pushed fixing the nit that @grondo noted above. removed WIP, will set MWP

chu11 avatar Aug 17 '22 22:08 chu11