-
Notifications
You must be signed in to change notification settings - Fork 529
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
fix: test_source_osv.py::TestSourceOSV::test_update_ecosystems fails because ecosystems.txt and self.osv.update_ecosystems mismatch #4633
Comments
Ugh, thanks for investigating. Looks like we need an update. |
Signed-off-by: weichslgartner <[email protected]>
@terriko from pathlib import Path
# gsutil ls gs://osv-vulnerabilities/Alpine > alpine.txt
alpine = Path("/tmp/osv/alpine.txt").open().readlines()
# gsutil ls gs://osv-vulnerabilities/Alpine:v3.10/ > alpine3_10.txt
alpine_3_10 = Path("/tmp/osv/alpine3_10.txt").open().readlines()
alpine_3_10_set = set(map(lambda x: x.split("/")[-1].strip(),alpine_3_10))
alpine_set = set(map(lambda x: x.split("/")[-1].strip(),alpine))
print(alpine_3_10_set.issubset(alpine_set)) # prints True
print(alpine_set.issubset(alpine_3_10_set)) # prints False |
Looking into this! |
Hi, @terriko I've identified and fixed the failing test in test_source_osv. py: TestSourceOSV: test_update_ecosystems. The failure was due to a mismatch between ecosystems.txt and the gsutil output, mainly caused by the [EMPTY] entry and versioned ecosystem names. I updated the test to remove [EMPTY], add missing parent ecosystems (DWF, JavaScript), and ensure versioned entries are mapped correctly. The fix improves consistency and performance by relying on ecosystems.txt for selection. I've opened a PR to fix this. Please review it when you get a chance. Let me know if any changes are needed. Thanks! 🚀 |
This fixes the test, but I think that the download of all ecosystems with gsutils is a lot redundant work as the toplevel ecosystems should be enough (as alpine 3:16 is a subset of alpine, alpine should be enough). |
Hi @weichslgartner , I have made the change you asked for. Also, I have moved enforcement of DWF and JS for when the external system is in use and the empty entry is discarded in sources. This allows us to enable the test locally if you want that. However, that isn't very beneficial, so I have left the skip test on no external system. The PR is now ready for @terriko to review. Please feel free to let me know if there's anything else that needs to be adjusted. |
Description
As https://github.com/intel/cve-bin-tool/actions/runs/12399617734/job/34614942113 failed, I looked if I could reproduce this locally and I had the same failure. The test which fails is
cve-bin-tool/test/test_source_osv.py
Line 170 in 707d110
The issue is that the unit tests compares the content of https://osv-vulnerabilities.storage.googleapis.com/ecosystems.txt against the ecosystems provided via the gsutil which is used as data source by cve-bin-tool.
The
ecosystems.txt
contains the entry[EMPTY]
which is not inexpected_ecosystems
. On the other side gsutil has values likeAlmaLinux:8
,AlmaLinux:9
while the txt file only containsAlmaLinux
.From the debugger I got the following values:
To reproduce
Steps to reproduce the behaviour (in the test folder):
EXTERNAL_SYSTEM=1 pytest test_source_osv.py::TestSourceOSV::test_update_ecosystems
Expected behaviour: Test passes
Actual behaviour: Test fails
Version/platform info
Version of CVE-bin-tool( e.g. output of
cve-bin-tool --version
): 3.4 (main branch)Installed from pypi or github? github
Operating system: Linux/Windows (other platforms are unsupported but feel free to report issues anyhow)
5.4.0-200-generic #220-Ubuntu
Python version (e.g.
python3 --version
): Python 3.9.18Running in any particular CI environment we should know about? (e.g. Github Actions)
Anything else?
If fixing the unit test is enough then I can provide this (local fix works). If it has other implications and needed changes in the implementation (should all the zip files of the dedicated version be downloaded or not? etc.) I need advice here.
The text was updated successfully, but these errors were encountered: