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

Misc KVS cleanup & minor fixes #1213

Merged
merged 8 commits into from
Sep 27, 2017
Merged

Conversation

chu11
Copy link
Member

@chu11 chu11 commented Sep 27, 2017

Pulling out these cleanup & minor fix patches from another patch series for #1187. Includes simple fixes (comments, code styles), minor cleanup (putting things in functions, removing unused parameters in function calls), some corner case fixes, and returning error on bad inputs to some functions.

Cleanup cache tests, placing tests in functions to make clear
tests being performed.
Remove unnecessary parameters passed to check() and check_stall()
functions.  The inputs are always the same, so they can be
abstracted away.
Remove unnecessary/unused parameter in load() function.
Add several corner case checks and add unit tests to increase
coverage.
When input is bad or not appropriate, return -1 in cache_entry_set_dirty()
and cache_entry_set_json().

Add/update unit tests appropriately.
@garlick
Copy link
Member

garlick commented Sep 27, 2017

All looks pretty reasonable to me.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.04%) to 78.632% when pulling f7a8984 on chu11:misccleanup9 into aa3cac0 on flux-framework:master.

@chu11
Copy link
Member Author

chu11 commented Sep 27, 2017

hit #1169, restarting a run

@garlick
Copy link
Member

garlick commented Sep 27, 2017

I'll check back later and merge this if it's green. (Have to duck out)

@codecov-io
Copy link

Codecov Report

Merging #1213 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1213      +/-   ##
==========================================
- Coverage   78.25%   78.23%   -0.03%     
==========================================
  Files         158      158              
  Lines       29139    29142       +3     
==========================================
- Hits        22804    22800       -4     
- Misses       6335     6342       +7
Impacted Files Coverage Δ
src/modules/kvs/cache.c 91.22% <ø> (ø) ⬆️
src/modules/kvs/kvs.c 64.11% <100%> (+0.43%) ⬆️
src/modules/kvs/lookup.c 85.03% <100%> (+0.01%) ⬆️
src/modules/connector-local/local.c 76.07% <0%> (-1.31%) ⬇️
src/common/libflux/request.c 87.17% <0%> (-1.29%) ⬇️
src/common/libflux/mrpc.c 85.49% <0%> (-1.18%) ⬇️
src/common/libflux/future.c 88.31% <0%> (-0.94%) ⬇️
src/common/libutil/base64.c 95.07% <0%> (-0.71%) ⬇️
src/common/libkvs/kvs_watch.c 86.78% <0%> (-0.45%) ⬇️
src/broker/overlay.c 71.92% <0%> (-0.36%) ⬇️
... and 6 more

@garlick garlick merged commit 92a797f into flux-framework:master Sep 27, 2017
@grondo grondo mentioned this pull request May 10, 2018
@chu11 chu11 deleted the misccleanup9 branch June 5, 2021 16:57
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.

4 participants