job-list: misc cleanup #5407
Merged
job-list: misc cleanup #5407
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Problem: In job-list/test/job_data.c, the function read_file() returns a file descriptor that no one uses. Have read_file() return void instead of the file descriptor. Simply close the file descriptor before returning.
Problem: In the header file job_data.h, the comments for what fields are parsed in specific parsing functions is out of date. Update the headers appropriately for all fields parsed by those functions.
chu11
force-pushed
the
job_list_misc_cleanup
branch
from
62c5633
to
9d48a7a
Compare
garlick
approved these changes
Aug 23, 2023
| @@ -266,13 +266,12 @@ static int parse_jobspec (struct job *job, const char *s, bool allow_nonfatal) | |||
| json_error_t error; | |||
There was a problem hiding this comment.
In the commit message for "job-list: do not return error when nonfatal set" there's a duplicate jobspec word: "illegal json jobspec jobspec"
There was a problem hiding this comment.
oops! thanks. i'll fix up then set MWP.
chu11
force-pushed
the
job_list_misc_cleanup
branch
from
9d48a7a
to
2069ce2
Compare
Problem: The internal function parse_jobspec() returns an error for an illegal json jobspec when the 'allow_nonfatal' flag is set. Do not return on error when 'allow_nonfatal' is set to true.
Problem: There are several tests in t2260-job-list.t that test that job-list works if R is illegal. However, they only test when R is an illegal formatted json object. They do not test if R is missing expected fields. Add additional tests that set R to an empty json object.
Problem: In the internal state transition struct, the flag "processed" is a little ambiguous. In addition, multiple locations manually remove a state transition from the next_states list. Solution: Split the "processed" flag into two flags, "processing" and "finished". The "processing" flag more clearly indicates that the state transition is being processed. The "finished" flag indicates the transition is done and we can remove the struct from the next_states list. With these new flags, we no longer need to remove the struct from the next_states list in so many locations. Simply set the "finished" flag to true and remove from the list in process_next_state().
chu11
force-pushed
the
job_list_misc_cleanup
branch
from
2069ce2
to
cc20c39
Compare
Codecov Report
@@ Coverage Diff @@
## master #5407 +/- ##
=======================================
Coverage 83.54% 83.55%
=======================================
Files 470 470
Lines 78577 78582 +5
=======================================
+ Hits 65646 65658 +12
+ Misses 12931 12924 -7
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
just splicing out some random cleanup before the bigger job-list PR that's upcoming.