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

python: fix return from flux.importer.import_plugins() when no plugins found #4288

Merged
merged 2 commits into from Apr 17, 2022

Conversation

grondo
Copy link
Contributor

@grondo grondo commented Apr 16, 2022

Problem: When flux.importer.import_plugins() does not find any
plugins to import, it returns an empty list, but the function should
always return a dict.

Return an empty dict when no plugins are found in import_plugins().

As discovered by @wkharold in #4285

Problem: When flux.importer.import_plugins() does not find any
plugins to import, it returns an empty list, but the function should
always return a dict.

Return an empty dict when no plugins are found in import_plugins().
Copy link
Member

@garlick garlick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

Problem: There is no test that ensures the flux.importer.import_plugins
function, used by the job validator, returns a dict and not a list when
there are no plugins found.

Because there is no import_plugins() unit test, simply add a specific
test to t2110-job-ingest-validator.t which verifies this case.
@grondo
Copy link
Contributor Author

grondo commented Apr 16, 2022

Felt bad that there was no test here, so I added a simple one to the job-validator sharness tests.

@codecov
Copy link

codecov bot commented Apr 16, 2022

Codecov Report

Merging #4288 (3860493) into master (91476ea) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master    #4288   +/-   ##
=======================================
  Coverage   83.56%   83.56%           
=======================================
  Files         388      388           
  Lines       64848    64848           
=======================================
+ Hits        54189    54193    +4     
+ Misses      10659    10655    -4     
Impacted Files Coverage Δ
src/bindings/python/flux/importer.py 95.83% <100.00%> (+8.33%) ⬆️
src/common/libpmi/pmi.c 91.81% <0.00%> (-1.82%) ⬇️
src/modules/job-archive/job-archive.c 62.13% <0.00%> (-0.74%) ⬇️
src/broker/state_machine.c 82.66% <0.00%> (-0.40%) ⬇️
src/broker/runat.c 83.33% <0.00%> (-0.33%) ⬇️
src/broker/overlay.c 86.69% <0.00%> (-0.11%) ⬇️
src/common/libterminus/terminus.c 86.06% <0.00%> (+0.24%) ⬆️
src/modules/cron/cron.c 82.92% <0.00%> (+0.44%) ⬆️
src/common/libkvs/kvs_commit.c 78.02% <0.00%> (+1.09%) ⬆️
src/common/libpmi/pmi2.c 89.87% <0.00%> (+1.26%) ⬆️
... and 2 more

Copy link
Member

@garlick garlick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, even nicer.

@mergify mergify bot merged commit 77f1708 into flux-framework:master Apr 17, 2022
@grondo grondo deleted the importer-return-dict branch April 17, 2022 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants