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

async utils: aio_run_command(): check returncode #1181

Merged

Conversation

pdxjohnny
Copy link
Member

No description provided.

@codecov-commenter
Copy link

codecov-commenter commented Jun 16, 2021

Codecov Report

Merging #1181 (92851d7) into main (420886a) will decrease coverage by 2.33%.
The diff coverage is 3.57%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1181      +/-   ##
==========================================
- Coverage   79.81%   77.47%   -2.34%     
==========================================
  Files         216      217       +1     
  Lines        4141     4169      +28     
  Branches      515      517       +2     
==========================================
- Hits         3305     3230      -75     
- Misses        705      798      +93     
- Partials      131      141      +10     
Flag Coverage Δ
longtests 77.47% <3.57%> (-2.34%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
test/test_async_utils.py 0.00% <0.00%> (ø)
cve_bin_tool/async_utils.py 88.23% <33.33%> (-1.42%) ⬇️
cve_bin_tool/checkers/xml2.py 61.36% <0.00%> (-27.28%) ⬇️
test/test_json.py 68.96% <0.00%> (-20.69%) ⬇️
test/test_scanner.py 61.71% <0.00%> (-12.50%) ⬇️
test/test_cli.py 81.28% <0.00%> (-11.83%) ⬇️
cve_bin_tool/checkers/systemd.py 85.18% <0.00%> (-7.41%) ⬇️
cve_bin_tool/checkers/glibc.py 96.29% <0.00%> (-3.71%) ⬇️
cve_bin_tool/extractor.py 60.15% <0.00%> (-3.13%) ⬇️
cve_bin_tool/cli.py 75.30% <0.00%> (-3.09%) ⬇️
... and 3 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 420886a...92851d7. Read the comment docs.

@pdxjohnny pdxjohnny requested a review from terriko June 16, 2021 21:31
Copy link
Contributor

@BreadGenie BreadGenie left a comment

Choose a reason for hiding this comment

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

Don't we need to add the new test file to the CI?

Copy link
Contributor

@terriko terriko left a comment

Choose a reason for hiding this comment

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

Yup, looks like we need to add the new test file to CI so the coverage will actually update correctly. Otherwise looks good to go, though.

@pdxjohnny pdxjohnny force-pushed the async_utils_aio_run_command_check_return_code branch from 5a8ea14 to b9bf983 Compare June 30, 2021 16:38
pdxjohnny and others added 2 commits June 30, 2021 09:38
Raise subprocess.CalledProcessError if subprocess fails with a non-zero
return code.

Fixes: intel#1177
Signed-off-by: John Andersen <johnandersenpdx@gmail.com>
@terriko terriko merged commit 94e37fd into intel:main Jun 30, 2021
terriko added a commit to terriko/cve-bin-tool that referenced this pull request Jul 28, 2021
Changes in intel#1181 to check return codes made it so failed extractions
caused cve-bin-tool to halt rather than log an error. This was
particularly a problem for .exe files, which we try to extract as
zipfiles in case they are self-extracting exectables.

Signed-off-by: Terri Oda <terri.oda@intel.com>
terriko added a commit to terriko/cve-bin-tool that referenced this pull request Jul 28, 2021
Changes in intel#1181 to check return codes made it so failed extractions
caused cve-bin-tool to halt rather than log an error. This was
particularly a problem for .exe files, which we try to extract as
zipfiles in case they are self-extracting exectables.

Signed-off-by: Terri Oda <terri.oda@intel.com>
terriko added a commit to terriko/cve-bin-tool that referenced this pull request Jul 29, 2021
Changes in intel#1181 to check return codes made it so failed extractions
caused cve-bin-tool to halt rather than log an error. This was
particularly a problem for .exe files, which we try to extract as
zipfiles in case they are self-extracting exectables.

Signed-off-by: Terri Oda <terri.oda@intel.com>
terriko added a commit that referenced this pull request Aug 2, 2021
* fixes #1281
* Related to #1181 

Changes in #1181 to check return codes made it so that cve-bin-tool failed if any file did not extract correctly. 

This changes the default so that failures during extraction are logged but do not halt the scan.  Error messages are not logged when the file extension is .exe because users found those confusing (cve-bin-tool tries to unzip all .exe files in case they are self-extracting zipfiles, but many are not)

Signed-off-by: Terri Oda <terri.oda@intel.com>
terriko added a commit that referenced this pull request Aug 4, 2021
* fixes #1281
* Related to #1181 

Changes in #1181 to check return codes made it so that cve-bin-tool failed if any file did not extract correctly. 

This changes the default so that failures during extraction are logged but do not halt the scan.  Error messages are not logged when the file extension is .exe because users found those confusing (cve-bin-tool tries to unzip all .exe files in case they are self-extracting zipfiles, but many are not)

Signed-off-by: Terri Oda <terri.oda@intel.com>
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