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

t2000-wreck.t: fix logic in check for archive job #1109

Merged
merged 1 commit into from Jul 18, 2017

Conversation

Projects
None yet
4 participants
@chu11
Copy link
Contributor

chu11 commented Jul 18, 2017

The check for the last complete job via

flux kvs dir lwj-complete | tail -1

is not safe b/c there is no guarantee that the directory entries
are sorted when output. In combination with the lastjobid, the
test may try to retrieve a kvs field that will never exist.

Sort lwj complete entries by epoch and then use tail -1. Adjust
surrounding code appropriately.

@chu11 chu11 force-pushed the chu11:wreckfix branch from a8bbe4b to 65043fe Jul 18, 2017

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Jul 18, 2017

The problem in #1101 is already fixed and closed. Can we either open a new issue with the specific problem this fixes, or just remove the Fixes reference in this commit message?

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jul 18, 2017

Coverage Status

Coverage increased (+0.007%) to 78.49% when pulling 65043fe on chu11:wreckfix into a3add43 on flux-framework:master.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jul 18, 2017

Codecov Report

Merging #1109 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1109   +/-   ##
=======================================
  Coverage   78.22%   78.22%           
=======================================
  Files         157      157           
  Lines       26132    26132           
=======================================
  Hits        20441    20441           
  Misses       5691     5691
Impacted Files Coverage Δ
src/common/libflux/future.c 87.16% <0%> (-1.07%) ⬇️
src/common/libflux/message.c 81.33% <0%> (+0.11%) ⬆️
src/common/libutil/dirwalk.c 94.07% <0%> (+0.74%) ⬆️
@chu11

This comment has been minimized.

Copy link
Contributor Author

chu11 commented Jul 18, 2017

@grondo Oops, we raced on messages b/c I forgot to click comment on #1101. I'll go ahead and open a new one to discuss the differences.

t2000-wreck.t: fix logic in check for archive job
The check for the last complete job via

flux kvs dir lwj-complete | tail -1

is not safe b/c there is no guarantee that the directory entries
are sorted when output.  In combination with the lastjobid, the
test may try to retrieve a kvs field that will never exist.

Sort lwj complete entries by epoch and then use tail -1.  Adjust
surrounding code appropriately.

Fixes #1110

@chu11 chu11 force-pushed the chu11:wreckfix branch from 65043fe to cc27972 Jul 18, 2017

@grondo

This comment has been minimized.

Copy link
Contributor

grondo commented Jul 18, 2017

Thanks, sorry. I thought I had missed your message.
FYI - in my debugging of #1101, at least when I recreated the issue, the problem was definitely that the latest epoch directory hadn't appeared in flux-kvs dir on the first iteration through the loop. I never did see the directories output in a different order such that the tail picked the wrong directory when the correct one was there.

I can definitely believe the directories might come out in random order, so I think this fix is correct. I just never saw it before.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jul 18, 2017

Coverage Status

Coverage increased (+0.03%) to 78.509% when pulling 65043fe on chu11:wreckfix into a3add43 on flux-framework:master.

@coveralls

This comment has been minimized.

Copy link

coveralls commented Jul 18, 2017

Coverage Status

Coverage remained the same at 78.482% when pulling cc27972 on chu11:wreckfix into a3add43 on flux-framework:master.

@grondo grondo merged commit 5776aeb into flux-framework:master Jul 18, 2017

4 checks passed

codecov/patch Coverage not affected when comparing a3add43...cc27972
Details
codecov/project 78.22% remains the same compared to a3add43
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage remained the same at 78.482%
Details

@grondo grondo referenced this pull request Aug 23, 2017

Closed

0.8.0 Release #1160

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.