Skip to content

Conversation

@arnavk23
Copy link
Contributor

@arnavk23 arnavk23 commented Nov 22, 2025

Reference Issues/PRs

Fixes #114.

What does this implement/fix? Explain your changes.

Isolate the lookup tests from the main package by introducing a dedicated test-only package skbase.tests.mock_package. The lookup tests now scan this small controlled package instead of skbase, preventing unrelated production refactors from breaking lookup tests.

Does your contribution introduce a new dependency? If yes, which one?

What should a reviewer concentrate their feedback on?

Any other comments?

PR checklist

For all contributions
  • I've reviewed the project documentation on contributing
  • I've added myself to the list of contributors.
  • The PR title starts with either [ENH], [CI/CD], [MNT], [DOC], or [BUG] indicating whether
    the PR topic is related to enhancement, CI/CD, maintenance, documentation, or a bug.
For code contributions
  • Unit tests have been added covering code functionality
  • Appropriate docstrings have been added (see documentation standards)
  • New public functionality has been added to the API Reference

@arnavk23 arnavk23 changed the title tests(lookup): isolate lookup tests to dedicated mock package [ENH] tests(lookup): isolate lookup tests to dedicated mock package Nov 22, 2025
__author__: List[str] = ["fkiraly", "RNKuhns"]


class Parent(BaseObject):

Check warning

Code scanning / CodeQL

`__eq__` not overridden when adding attributes Warning test

The class 'Parent' does not override
'__eq__'
, but adds the new attribute
a
.
The class 'Parent' does not override
'__eq__'
, but adds the new attribute
b
.
The class 'Parent' does not override
'__eq__'
, but adds the new attribute
c
.
@codecov
Copy link

codecov bot commented Dec 8, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 83.72%. Comparing base (306958d) to head (a8946f8).
⚠️ Report is 171 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #476      +/-   ##
==========================================
- Coverage   85.07%   83.72%   -1.36%     
==========================================
  Files          45       52       +7     
  Lines        3015     3889     +874     
==========================================
+ Hits         2565     3256     +691     
- Misses        450      633     +183     

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

@arnavk23 arnavk23 force-pushed the fix/issue-114-restrict-lookup-tests branch 2 times, most recently from 0c30a63 to 6e00981 Compare December 15, 2025 15:45
arnavk23 and others added 7 commits December 15, 2025 21:19
…n mock_package modules

Classes defined in test_fixtures.py should only appear in that module's
metadata, not in __init__.py or test_mock_package.py where they're imported.
Removed Parent, Child, ClassWithABTrue from __all__ lists to match the test's
expectation that get_package_metadata only reports classes actually defined
in each module (cls.__module__ == module.__name__).
…ckage

Classes are imported for re-export to maintain the module's public API
but are intentionally not listed in __all__ to ensure get_package_metadata
only reports classes defined in each module.
@arnavk23
Copy link
Contributor Author

@fkiraly all tests pass and fixed the pre-commit failing check by making change to test_base.py to comply with flake8

@arnavk23
Copy link
Contributor Author

arnavk23 commented Jan 4, 2026

@fkiraly all tests pass and the pr is rebased.

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.

Reducing scope of test_get_package_metadata_returns_expected_results?

2 participants