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

MINOR: Rewrite unchecked operations in Mock API #19071

Open
wants to merge 6 commits into
base: trunk
Choose a base branch
from

Conversation

clarkwtc
Copy link
Contributor

@clarkwtc clarkwtc commented Mar 2, 2025

We encountered unchecked or unsafe operations in GroupMetadataManagerTest.java and KTableImplTest.java.

  • Rewrite getArgument of invocation in InvocationOnMock API because the implementation of InvocationOnMock discards type anyway.

    • Before
    Screenshot 2025-03-02 at 8 50 55 AM
    • After
    Screenshot 2025-03-02 at 8 51 05 AM
  • Remove unchecked annotations for using mock API without variable assignment.

Follow-up: mockito/mockito#1609

…se the implementation of InvocationOnMock discards type anyway.
@github-actions github-actions bot added triage PRs from the community tests Test fixes (including flaky tests) small Small PRs labels Mar 2, 2025
clarkwtc added 2 commits March 2, 2025 10:18
…I, because the implementation of InvocationOnMock discards type anyway."

This reverts commit ef6d475.
…se the implementation of InvocationOnMock discards type anyway.
@clarkwtc clarkwtc changed the title MINOR: Rewrite getArgument of invocation to ommit the class-argument MINOR: Rewrite unchecked operations in Mock API Mar 2, 2025
Copy link
Contributor

@m1a2st m1a2st left a comment

Choose a reason for hiding this comment

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

Thank @clarkwtc for this patch, left a comment

Comment on lines -593 to +592
final ValueTransformerWithKeySupplier<String, String, ?> valueTransformerSupplier =
mock(ValueTransformerWithKeySupplier.class);
final ValueTransformerWithKeySupplier<String, String, ?> valueTransformerSupplier = mock();
Copy link
Contributor

@m1a2st m1a2st Mar 2, 2025

Choose a reason for hiding this comment

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

I use ./gradlew clean build -x test command on this branch, It still have the warning message
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for checking again.
I’ve fixed it.

Copy link
Contributor

@frankvicky frankvicky left a comment

Choose a reason for hiding this comment

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

Why change the type to var keyword?

@clarkwtc
Copy link
Contributor Author

clarkwtc commented Mar 2, 2025

Why change the type to var keyword?

@frankvicky
Screenshot 2025-03-02 at 5 17 47 PM

This change is meant to resolve the unchecked cast for KTableImpl.

@frankvicky
Copy link
Contributor

@clarkwtc
I don't think it must use var.
For example:

    @Test
    public void testStateStore() {
        final StreamsBuilder builder = new StreamsBuilder();
        final String topic1 = "topic1";
        final String topic2 = "topic2";

        final KTable<String, String> table1 = builder.table(topic1, consumed);
        final KTable<String, String> table2 = builder.table(topic2, consumed);

        final KTable<String, Integer> table1Mapped = table1.mapValues(s -> Integer.valueOf(s));
        final KTable<String, Integer> table1MappedFiltered = table1Mapped.filter((key, value) -> (value % 2) == 0);
        table2.join(table1MappedFiltered, (v1, v2) -> v1 + v2);

        try (final TopologyTestDriver driver = new TopologyTestDriver(builder.build(), props)) {
            assertEquals(2, driver.getAllStateStores().size());
        }
    }

In this way, there will be no warning during the build.

@github-actions github-actions bot removed the triage PRs from the community label Mar 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci-approved small Small PRs streams tests Test fixes (including flaky tests)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants