MINOR: Exclude '**/*Suite.class' from test, unitTest and integrationTest

[chia7712]

./gradlew unitTest integrationTest \
    --profile --no-daemon --continue -PtestLoggingEvents=started,passed,skipped,failed "$@" \
    || { echo 'Test steps failed'; exit 1; }

The tasks unitTest and integrationTest used to run tests don't exclude the **/*Suite so the tests included by Suite class are executed two times. For example:

11:42:25 org.apache.kafka.streams.integration.StoreQuerySuite > org.apache.kafka.streams.integration.QueryableStateIntegrationTest.shouldBeAbleToQueryMapValuesState STARTED
11:42:26 
11:42:26 org.apache.kafka.streams.integration.GlobalKTableIntegrationTest > shouldKStreamGlobalKTableJoin PASSED
11:42:30 
11:42:30 org.apache.kafka.streams.integration.StoreQuerySuite > org.apache.kafka.streams.integration.QueryableStateIntegrationTest.shouldBeAbleToQueryMapValuesState PASSED
...
11:48:42 org.apache.kafka.streams.integration.QueryableStateIntegrationTest > shouldBeAbleToQueryMapValuesState STARTED
11:48:46 
11:48:46 org.apache.kafka.streams.integration.QueryableStateIntegrationTest > shouldBeAbleToQueryMapValuesState PASSED

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[chia7712]

@ijuma FYI

[ijuma]

Good catch, we should do the same for the test block, right? @guozhangwang @mjsax why do we have these suite classes in the first place?

[chia7712]

we should do the same for the test block, right?

It was addressed by #5527 (the rule is added to streams module only since the Suite.class is used by streams module only)

PR
https://github.com/apache/kafka/pull/5527/files#diff-c197962302397baf3a4cc36463dce5eaR1228
current code
https://github.com/apache/kafka/blob/trunk/build.gradle#L1283

[ijuma]

I think we should do it consistently at the top level

[chia7712]

I think we should do it consistently at the top level

done

[mjsax]

why do we have these suite classes in the first place?

@vvcephei Suggested to add them and I think they are useful if one works on a specific feature (like suppress(), FK-joins, EOS): you can run all relevant test at once in IntelliJ in a convenient way.

I agree that we should not run the test twice in the build though.

[vvcephei]

Ah, my mistake with the incorrect scoping. Thanks for the fix.

FWIW, I do find that putting together a Suite is very useful while doing feature development, but having the suites checked in is less useful.

Coming back to features I’ve made suites for, I’ve often had to add or remove tests from the suites to cover the logic I’m currently working on.

Now, I’m wondering if we should just delete the Suite files and not allow them to be checked in, but only use them for local dev.

[ijuma]

LGTM

[ijuma]

@vvcephei I would agree, but I'll merge this PR as is since it solves the issue and doesn't cause harm even if we remove the Suite classes. Feel free to submit a separate PR to remove the Suite classes if there is consensus on that.

[chia7712]

QueryableStateIntegrationTest.concurrentAccesses seems to be a flaky.

[chia7712]

retest this please