Skip to content

Conversation

@another-rex
Copy link
Collaborator

Pin the python dependencies so it doesn't change and break our cassettes

@another-rex another-rex requested review from Ly-Joey and cuixq December 1, 2025 00:20
@G-Rath
Copy link
Collaborator

G-Rath commented Dec 1, 2025

doesn't this defeat the point though, as we're wanting to test transient resolving?

This is why some of the other Python tests don't use cassettes

@another-rex
Copy link
Collaborator Author

I don't think that's this test, this is for testing scanning artifacts in image scanning, so the transitive resolution is being done by pip, not by osv-scanner.

@cuixq
Copy link
Contributor

cuixq commented Dec 1, 2025

@G-Rath the test in change is for image scanning but not source scanning

@G-Rath
Copy link
Collaborator

G-Rath commented Dec 1, 2025

Sure, but it still tests our dealings with external package managers right? I'm not saying this is completely wrong, I'm just checking that has been discussed especially as there is an alternative action we could take (not use cassettes for this particular test) 🙂

@G-Rath
Copy link
Collaborator

G-Rath commented Dec 1, 2025

also I just realized the pull request title checker doesn't seem to be running - I think its not running for PRs from forks?

(tests is not a valid conventional commit type, it should be test)

@another-rex
Copy link
Collaborator Author

also I just realized the pull request title checker doesn't seem to be running - I think its not running for PRs from forks?

(tests is not a valid conventional commit type, it should be test)

Huh that is weird, I'm sure it was running before for my PRs which have always been forks.

@G-Rath
Copy link
Collaborator

G-Rath commented Dec 1, 2025

yeah and I just opened #2381 where it ran, but it doesn't look like it ran on #2234?

@another-rex
Copy link
Collaborator Author

Sure, but it still tests our dealings with external package managers right? I'm not saying this is completely wrong, I'm just checking that has been discussed especially as there is an alternative action we could take (not use cassettes for this particular test) 🙂

For this case I think it's fine, I've been wanting to pin the dependencies here for a while, just haven't gotten around to it. The images that we are testing should not be changing over time.

@codecov-commenter
Copy link

codecov-commenter commented Dec 1, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 67.68%. Comparing base (cb0fd76) to head (1a120f9).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2380   +/-   ##
=======================================
  Coverage   67.68%   67.68%           
=======================================
  Files         172      172           
  Lines       13147    13147           
=======================================
  Hits         8898     8898           
  Misses       3551     3551           
  Partials      698      698           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@another-rex another-rex merged commit 7468bcf into google:main Dec 1, 2025
16 checks passed
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.

5 participants