Skip to content
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

Improved IntegratedFixture and static check cleanups (C4-932) #222

Merged
merged 6 commits into from
Sep 30, 2022

Conversation

netsettler
Copy link
Collaborator

  • Show less uninteresting traceback info for static tests.
  • Small incompatible change to recently released qa_checkers.confirm_no_uses to remove the if_used argument in favor of a simpler implementation. Since this is testing-only, not something used in production, and since there are believed to be no uses outside the repo, no major version bump.
  • Make class initialization of IntegratedFixture happen at instance instantiation time. That simplifies the loading actions needed. Those can happen in conftest.py rather than in ff_mocks, which should allow dcicutils.ff_mocks to be imported more easily (C4-932).

…A will do. Update qa_checkers to do type declarations on WARNING_CATEGORY in two classes.
@coveralls
Copy link

coveralls commented Sep 30, 2022

Pull Request Test Coverage Report for Build 3161551459

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 90 unchanged lines in 2 files lost coverage.
  • Overall coverage increased (+0.05%) to 71.365%

Files with Coverage Reduction New Missed Lines %
dcicutils/qa_checkers.py 37 69.65%
dcicutils/ff_mocks.py 53 62.24%
Totals Coverage Status
Change from base Build 3146164003: 0.05%
Covered Lines: 5570
Relevant Lines: 7805

💛 - Coveralls

Copy link
Member

@willronchetti willronchetti left a comment

Choose a reason for hiding this comment

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

Looks good, just one small clarifying question

warnings.warn(message, category=warning_category, stacklevel=2)
else: # if_used == 'error'
raise AssertionError(message)
raise AssertionError(message)
Copy link
Member

Choose a reason for hiding this comment

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

Was the if_used behavior here removed intentionally? If so docstring should be updated

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, it's intentional. It was recently added and no one is using it yet. I found a place higher in the call chain to do this effect more simply. I'll adjust doc string.

@netsettler netsettler changed the title Improved IntegratedFIxture and static check cleanups Improved IntegratedFixture and static check cleanups Sep 30, 2022
@netsettler netsettler changed the title Improved IntegratedFixture and static check cleanups Improved IntegratedFixture and static check cleanups (C4-932) Sep 30, 2022
@netsettler netsettler merged commit 811bc45 into master Sep 30, 2022
@netsettler netsettler deleted the kmp_better_integrated_fixture branch March 30, 2023 17:50
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.

3 participants