-
Notifications
You must be signed in to change notification settings - Fork 39
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
Apply assorted test cleanup #562
Conversation
Looks good. No mutations were possible for these changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had eye-balled these for some time now; finally decided to flush them.
private final CompilationTestHelper compilationTestHelper = | ||
CompilationTestHelper.newInstance(RedundantStringConversion.class, getClass()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that initially we decided against this, but it is more consistent this way (and it may help simplify the generalizations we're discussing in #494).
private final CompilationTestHelper compilationTestHelper = | ||
CompilationTestHelper.newInstance(TestChecker.class, getClass()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's also a BugCheckerRefactoringTestHelper
field in SourceCodeTest
, but that's part of #561.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice. I agree, it's fine to always inline them.
Summary of changes: - Inline more `CompilationTestHelper` fields. - Move inner class to the bottom of the outer class. - Improve test parameter name.
221f1d3
to
6498ced
Compare
Rebased, will merge once 🟢. |
Looks good. No mutations were possible for these changes. |
Suggested commit message: