Skip to content

fix: Database records being lost (fixes #3150) #3151

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

Merged
merged 6 commits into from
Jul 12, 2023

Conversation

anthonyharrison
Copy link
Contributor

No description provided.

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.

Looking good. As expected, it's causing one test to fail (though only one?!):

=========================== short test summary info ============================
FAILED test/test_cli.py::TestCLI::test_SBOM - AssertionError: assert ('cve_bin_tool', 20, 'There are 2 products with known CVEs detected') in [('cve_bin_tool', 20, 'CVE Binary Tool v3.2.2dev0'), ('cve_bin_tool', 20, 'This product uses the NVD API but is not endorsed or certified by the NVD.'), ('cve_bin_tool', 20, 'Not using an NVD API key. Your access may be rate limited by NVD.'), ('cve_bin_tool', 20, 'Get an NVD API key here: https://nvd.nist.gov/developers/request-an-api-key'), ('cve_bin_tool', 30, 'Using legacy JSON interface'), ('cve_bin_tool.CVEDB', 20, 'Using cached CVE data (<24h old). Use -u now to update immediately.'), ...]
 +  where [('cve_bin_tool', 20, 'CVE Binary Tool v3.2.2dev0'), ('cve_bin_tool', 20, 'This product uses the NVD API but is not endorsed or certified by the NVD.'), ('cve_bin_tool', 20, 'Not using an NVD API key. Your access may be rate limited by NVD.'), ('cve_bin_tool', 20, 'Get an NVD API key here: https://nvd.nist.gov/developers/request-an-api-key'), ('cve_bin_tool', 30, 'Using legacy JSON interface'), ('cve_bin_tool.CVEDB', 20, 'Using cached CVE data (<24h old). Use -u now to update immediately.'), ...] = <_pytest.logging.LogCaptureFixture object at 0x7fa98c1f08e0>.record_tuples
======= 1 failed, 24 passed, 2 skipped, 72 warnings in 604.54s (0:10:04) =======

This might be a good time to update those tests to use some issubset() set arithmetic or other logic that makes them more robust to new things showing up in scans.

@anthonyharrison
Copy link
Contributor Author

Looking good. As expected, it's causing one test to fail (though only one?!):

@terriko The error was due to the fact that an extra product was found due to a recent vulnerability. However, the vulnerability didn't appear when using the NVD API key interface and it only appeared when I used the JSON interface. I note that both the Python long tests and Window tests are using the JSON interface with the NVD.

I have updated the test to capture 3 products but I agree that the general checking of number of products/vulnerabilities in the tests need to be refactored to be more robust

@codecov-commenter
Copy link

codecov-commenter commented Jul 12, 2023

Codecov Report

Merging #3151 (e7ef7a8) into main (8108645) will increase coverage by 5.79%.
The diff coverage is 41.66%.

@@            Coverage Diff             @@
##             main    #3151      +/-   ##
==========================================
+ Coverage   76.15%   81.94%   +5.79%     
==========================================
  Files         716      716              
  Lines       11082    11084       +2     
  Branches     1488     1488              
==========================================
+ Hits         8439     9083     +644     
+ Misses       2316     1618     -698     
- Partials      327      383      +56     
Flag Coverage Δ
longtests 81.41% <41.66%> (+5.26%) ⬆️
win-longtests 79.54% <41.66%> (?)

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

Impacted Files Coverage Δ
cve_bin_tool/data_sources/nvd_source.py 61.04% <0.00%> (+38.86%) ⬆️
cve_bin_tool/cvedb.py 63.10% <100.00%> (+32.97%) ⬆️
test/test_cli.py 88.77% <100.00%> (+18.70%) ⬆️

... and 22 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@anthonyharrison
Copy link
Contributor Author

This PR needs the database to be rebuilt as the the Foreign Key for cve_range has changed. How do we do that without upsetting all of the other PRs?

@terriko
Copy link
Contributor

terriko commented Jul 12, 2023

Merge conflict resolved, but of course that means the tests will run again...

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.

CI is behaving so I'm going to go ahead and merge then update the cache now. As we discussed today, updating the cache may re-break the test.

@terriko terriko merged commit 0b9ce36 into intel:main Jul 12, 2023
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.

3 participants