xiangfu0 commented on code in PR #19856:
URL: https://github.com/apache/pinot/pull/19856#discussion_r4278294092
##########
pinot-integration-tests/pom.xml:
##########
@@ -230,27 +230,9 @@
<exclude>org/apache/pinot/integration/tests/KafkaConfluentSchemaRegistryAvroMessageDecoderRealtimeClusterIntegrationTest.java</exclude>
<exclude>org/apache/pinot/integration/tests/KafkaConsumingSegmentToBeMovedSummaryIntegrationTest.java</exclude>
<exclude>org/apache/pinot/integration/tests/LLCRealtimeClusterIntegrationTest.java</exclude>
-
<exclude>org/apache/pinot/integration/tests/MultiNodesOfflineClusterIntegrationTest.java</exclude>
</excludes>
</configuration>
<executions>
- <execution>
- <id>multi-nodes-offline-integration-test-suite</id>
Review Comment:
Dropping this dedicated execution is fine, but note the full inherited
surface now shares one fork with `testServerHardFailure`, which still carries a
`// TODO: This is a flaky test` race between server shutdown and the in-flight
query. It restores state in a `finally` ("Other tests may be affected"), so a
flake fails only that method rather than the class — just flagging that the
broader run slightly raises the chance of seeing that known flake in this lane.
##########
pinot-integration-tests/pom.xml:
##########
@@ -230,27 +230,9 @@
<exclude>org/apache/pinot/integration/tests/KafkaConfluentSchemaRegistryAvroMessageDecoderRealtimeClusterIntegrationTest.java</exclude>
<exclude>org/apache/pinot/integration/tests/KafkaConsumingSegmentToBeMovedSummaryIntegrationTest.java</exclude>
<exclude>org/apache/pinot/integration/tests/LLCRealtimeClusterIntegrationTest.java</exclude>
-
<exclude>org/apache/pinot/integration/tests/MultiNodesOfflineClusterIntegrationTest.java</exclude>
Review Comment:
Removing this exclusion is the crux of the change — the lane-B `M*Test.java`
default include now discovers `MultiNodesOfflineClusterIntegrationTest`
directly, running every enabled inherited test on the 2-broker/3-server
cluster. Worth a one-line comment somewhere (here or in the test class)
recording that this is intentional, so a future reader doesn't "helpfully"
re-add a narrower selection and silently drop coverage again like the old
`@SelectMethod` suite did.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
