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

Get rid of a TestCase class. #20656

Merged
merged 1 commit into from
Mar 8, 2024

Conversation

benjyw
Copy link
Contributor

@benjyw benjyw commented Mar 8, 2024

Turns its members into new-style standalone test_* functions
(and a couple of helpers).

The diff is large, but the change is purely mechanical: dedent the
functions and remove all references to self. The proof is that all
77 tests in this file pass after this change (I have verified
that no tests are missed by the pytest collector).

This is in preparation for integrating the rust options parser, which
may require adding test fixtures, which is not a feature available
to old-style class based tests.

@benjyw benjyw added the category:internal CI, fixes for not-yet-released features, etc. label Mar 8, 2024
@benjyw benjyw requested a review from huonw March 8, 2024 03:00
@benjyw
Copy link
Contributor Author

benjyw commented Mar 8, 2024

Sorry for the giant diff - I do not recommend reviewing line-by-line but taking in the principle of the thing. Again, the change was done via find-and-replace.

Copy link
Contributor

@huonw huonw left a comment

Choose a reason for hiding this comment

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

Cool!

The ignore-whitespace diff made this easier to review: https://github.com/pantsbuild/pants/pull/20656/files?diff=split&w=1

@benjyw
Copy link
Contributor Author

benjyw commented Mar 8, 2024

Cool!

The ignore-whitespace diff made this easier to review: https://github.com/pantsbuild/pants/pull/20656/files?diff=split&w=1

Good tip!

@benjyw benjyw merged commit 908d245 into pantsbuild:main Mar 8, 2024
24 checks passed
@benjyw benjyw deleted the remove_options_test_class branch March 8, 2024 04:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:internal CI, fixes for not-yet-released features, etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants