Skip to content

Package repository case insensitive error handling #2 #1974

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.

Sign up for GitHub

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

Jump to bottom
Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Package repository case insensitive error handling #2 #1974

wants to merge 10 commits into from

Conversation

BryceGattis
Copy link
Contributor

@BryceGattis BryceGattis commented May 15, 2025 •
edited
Loading

Remake of the original PR #1951 on my personal GitHub.

Closes #1931.

This adds a new unit test for the fix as well.

@BryceGattis
Add failing test case
011b52f 
Signed-off-by: BryceGattis <[email protected]>
@BryceGattis BryceGattis requested a review from a team as a code owner May 15, 2025 18:59
@BryceGattis
Fix flake8 issue
4c78a84 
Signed-off-by: BryceGattis <[email protected]>
@codecov Codecov
Copy link

codecov bot commented May 15, 2025 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 60.06%. Comparing base (4708523) to head (193a216).
Report is 1 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1974      +/-   ##
==========================================
+ Coverage   60.02%   60.06%   +0.04%     
==========================================
  Files         163      163              
  Lines       20118    20120       +2     
  Branches     3506     3507       +1     
==========================================
+ Hits        12075    12086      +11     
+ Misses       7223     7220       -3     
+ Partials      820      814       -6

☔ 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.

@JeanChristopheMorinPerso JeanChristopheMorinPerso mentioned this pull request May 15, 2025
Closed
JeanChristopheMorinPerso
JeanChristopheMorinPerso reviewed May 15, 2025
View reviewed changes
src/rez/tests/test_package_respository.py Outdated
case_mismatch_package = create_package("MyTestPackage", data={})
case_mismatch_variant = next(case_mismatch_package.iter_variants())

pkg_repository._create_variant(variant, overrides={})
Copy link
Member

Choose a reason for hiding this comment

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

Does this leave files behind?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't believe so. I will confirm. The TempdirMixin should handle it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Confirmed that the folder is cleaned up. There are some other unit tests that are not properly cleaning up their temp folders though. I will push an update to this PR.

@BryceGattis @JeanChristopheMorinPerso
Update src/rez/tests/test_package_respository.py
d0daca8 
Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Bryce Gattis <[email protected]>
JeanChristopheMorinPerso
JeanChristopheMorinPerso reviewed May 15, 2025
View reviewed changes
@BryceGattis @JeanChristopheMorinPerso
Update src/rez/tests/test_package_respository.py
3c68222 
Co-authored-by: Jean-Christophe Morin <[email protected]>
Signed-off-by: Bryce Gattis <[email protected]>
@BryceGattis
Copy link
Contributor Author

BryceGattis commented May 15, 2025 •
edited
Loading

@JeanChristopheMorinPerso One thing that I could use some help on, from poking around, I expected that adding this new test file would allow me to do rez selftest --package_repository however, I just get:

rez selftest --package_repository
usage: rez [-h] [-i] [-V] [-v] COMMAND ...
rez: error: unrecognized arguments: --package_repository

Looking at selftest, it should be autogenerated as a new flag. Do you have any idea why it's not working?

@JeanChristopheMorinPerso
Copy link
Member

@BryceGattis They should appear in rez-selftest if you add a top level docstring if I'm not mistaken.

@BryceGattis
Copy link
Contributor Author

@BryceGattis They should appear in rez-selftest if you add a top level docstring if I'm not mistaken.

It was due to a typo in the filename 🤦

@BryceGattis
Fix name of new test file
98f5d75 
Signed-off-by: BryceGattis <[email protected]>
@JeanChristopheMorinPerso JeanChristopheMorinPerso added the devdays25 ASWF Dev Days 2025 label May 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JeanChristopheMorinPerso JeanChristopheMorinPerso JeanChristopheMorinPerso left review comments

Assignees
No one assigned
Labels
devdays25 ASWF Dev Days 2025
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

rezplugins\package_repository\filesystem.py failing on Windows when looking for a package family with mismatching case.
2 participants
@BryceGattis @JeanChristopheMorinPerso