Skip to content

Add tests for package_search (#2092) - #2149

Open
ahlbertng wants to merge 8 commits into
AcademySoftwareFoundation:mainfrom
ahlbertng:add-package-search-tests
Open

Add tests for package_search (#2092)#2149
ahlbertng wants to merge 8 commits into
AcademySoftwareFoundation:mainfrom
ahlbertng:add-package-search-tests

Conversation

@ahlbertng

@ahlbertng ahlbertng commented Jul 8, 2026

Copy link
Copy Markdown

Description

This PR adds unit tests for src/rez/package_search.py, which currently has no test coverage (as noted in #2092).

Addresses part of #2092.

What's tested

get_reverse_dependency_tree()
Correctly builds a reverse dependency tree for a package with known direct dependents
Respects the depth argument (e.g. depth=0 returns only the root package)
ResourceSearcher.search()
Finds all versions of a package family
Returns only the latest version when latest=True
ResourceSearchResultFormatter
Default (no output_format) output correctly shows a package's qualified name
ResourceSearchResult
Correctly stores resource, resource_type, and validation_error (including default None)

Notes

Tests reuse the existing solver/packages test data already used by test_packages.py, so no new test fixtures were added.
get_plugins() is not yet covered, no existing test data currently includes packages with plugin_for set. Open to guidance on whether to add new fixture packages for this, or if there's a preferred approach.

@ahlbertng
ahlbertng requested a review from a team as a code owner July 8, 2026 02:04
@linux-foundation-easycla

linux-foundation-easycla Bot commented Jul 8, 2026

Copy link
Copy Markdown

CLA Signed
The committers listed above are authorized under a signed CLA.

@ahlbertng
ahlbertng force-pushed the add-package-search-tests branch from c11bd7b to 26ef581 Compare July 8, 2026 02:22
@codecov

codecov Bot commented Jul 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.72%. Comparing base (5c598c5) to head (caa9a77).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2149      +/-   ##
==========================================
+ Coverage   61.29%   61.72%   +0.43%     
==========================================
  Files         164      164              
  Lines       20568    20568              
  Branches     3575     3575              
==========================================
+ Hits        12607    12696      +89     
+ Misses       7089     6983     -106     
- Partials      872      889      +17     

☔ View full report in Codecov by Harness.
📢 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.

@maxnbk maxnbk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you for the submission of this new set of tests. We really appreciate any added coverage we can squeeze into the repo at this point, especially for such un-tested areas.

I've issued some specific comments attached to areas near the code that they pertain to. These are the main comments I'm making that would flip my recommendation from "don't merge" to "do merge". I made some specific line comments, but the real issue is that the current flake8 linter job fails, so just address those items.

I would express that the largest useful gaps to continue covering, whether they are a part of this PR, or you or anyone else tackles with additional work, would be these:

(Please comment, if you intend to go further with the coverage and aim to address these as well, or if you want to keep this PR self-contained just to the current change-set)


test the validate=True path. The ResourceSearcher.search() method has a substantial validate code path with error handling for ResourceContentError. No tests exercise this yet, and is arguably the most complex portion of the module.

test for glob patterns. ResourceSearcher.search() uses fnmatch.fnmatch on the name pattern. A test like searcher.search("py*") would verify the glob codepaths, which is a core feature of the searcher.

test for family resource type. When no version range is specified and multiple families match, the searcher returns resource_type="family". The current tests only exercise the package resource type.

Comment thread src/rez/tests/test_package_search.py
Comment thread src/rez/tests/test_package_search.py
Comment thread src/rez/tests/test_package_search.py
Comment thread src/rez/tests/test_package_search.py Outdated
@ahlbertng

Copy link
Copy Markdown
Author

Thanks for the detailed feedback! Fixed the flake8 issues (E302/E305 blank line spacing). It should be clean now.

@ahlbertng

Copy link
Copy Markdown
Author

I'd like to keep this PR focused on the current change set for now, since I'm still learning the code base. I am happy to open a follow up PR tackling the validate=True path, glob pattern tests, and family resource type tests next, once this one's merged

@maxnbk

maxnbk commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

If you wish, a rebase will clear the ruff CI error (fixed on main), otherwise I believe this is fine to merge.

@maxnbk
maxnbk self-requested a review August 1, 2026 19:54

@maxnbk maxnbk left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rebase to clear CI would be nice but otherwise good to merge.

@maxnbk maxnbk added this to the Next milestone Aug 1, 2026
@maxnbk maxnbk added the tests test-only PR label Aug 1, 2026
Signed-off-by: Ahlbertng <70105688+ahlbertng@users.noreply.github.com>
Signed-off-by: Ahlbertng <70105688+ahlbertng@users.noreply.github.com>
Signed-off-by: Ahlbertng <70105688+ahlbertng@users.noreply.github.com>
Signed-off-by: Ahlbertng <70105688+ahlbertng@users.noreply.github.com>
Signed-off-by: Ahlbertng <70105688+ahlbertng@users.noreply.github.com>
Signed-off-by: Ahlbertng <70105688+ahlbertng@users.noreply.github.com>
Signed-off-by: Ahlbertng <70105688+ahlbertng@users.noreply.github.com>
Signed-off-by: Ahlbertng <70105688+ahlbertng@users.noreply.github.com>
@ahlbertng
ahlbertng force-pushed the add-package-search-tests branch from 62c3eb3 to caa9a77 Compare August 1, 2026 22:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tests test-only PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants