Skip to content

Conversation

@JarvisCraft
Copy link

@JarvisCraft JarvisCraft commented Aug 28, 2025

Currently, UnusedMethod check does not recognize implicitly referenced JUnit @MethodSource, e.g. the ones which are derived from test method name when none are specified explicitly. This PR fixes this issue.

Fixes #5289

@google-cla
Copy link

google-cla bot commented Aug 28, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@JarvisCraft JarvisCraft force-pushed the junit-implicit-method-source branch 2 times, most recently from e74fbe1 to f67790b Compare August 28, 2025 13:34
@JarvisCraft JarvisCraft force-pushed the junit-implicit-method-source branch from f67790b to 457c855 Compare August 28, 2025 13:36
@JarvisCraft
Copy link
Author

Hi @cushon, @graememorgan; could you please run the CI on this PR?

@Pankraz76
Copy link

Pankraz76 commented Nov 5, 2025

nice plz include @After also this is kind of a general topic having to watch many actually all? junit annotations.

There will be new stuff to come for sure.

right @marcphilipp?

thank you.

/home/runner/work/OpenSearch/OpenSearch/test/framework/src/main/java/org/opensearch/search/aggregations/AggregatorTestCase.java:1370: error: [UnusedMethod] Method 'cleanupReleasables' is never used.
    private void cleanupReleasables() {

@marcphilipp
Copy link

marcphilipp commented Nov 5, 2025

@Pankraz76 @After is from JUnit 4 and such methods must be public.

// normalizing unset value to the empty value
.map(a -> getAnnotationValue(a, "value")
.map(y -> asStrings(y).map(state::getName).map(Name::toString).collect(toImmutableSet()))
.orElse(ImmutableSet.of())
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: could this be .orElse(ImmutableSet.of(sym.name.toString())) instead of separately checking of names is empty below?

Copy link
Author

@JarvisCraft JarvisCraft Nov 6, 2025

Choose a reason for hiding this comment

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

It couldn't, since we also want to handle the case @MethodSource({})

Copy link
Collaborator

@cushon cushon left a comment

Choose a reason for hiding this comment

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

Thanks, looks good overall!

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.

UnusedMethod not recognize Junit MethodSource

4 participants