You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "gharris1727 (via GitHub)" <gi...@apache.org> on 2024/02/09 20:25:54 UTC
[PR] KAFKA-16242: Mark tests with the most flaky failures as integration tests [kafka]
gharris1727 opened a new pull request, #15349:
URL: https://github.com/apache/kafka/pull/15349
These tests have had at least one flaky test failure in the last 28 days, and aren't already marked as integration tests.
I looked at the top-50 flakiest tests in the project with gradle enterprise, and then spot-checked the top-50 flakiest tests in core, connect, tools, and streams.
I've also temporarily changed the Jenkinsfile to only run the unit tests to get an idea of the runtime and success rate of the unit tests in Jenkins.
### Committer Checklist (excluded from commit message)
- [ ] Verify design and implementation
- [ ] Verify test coverage and CI build status
- [ ] Verify documentation (including upgrade notes)
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "C0urante (via GitHub)" <gi...@apache.org>.
C0urante commented on code in PR #15349:
URL: https://github.com/apache/kafka/pull/15349#discussion_r1485632038
##########
connect/mirror/src/test/java/org/apache/kafka/connect/mirror/integration/MirrorConnectorsIntegrationTransactionsTest.java:
##########
@@ -31,6 +32,7 @@
* Integration test for MirrorMaker2 in which source records are emitted with a transactional producer,
* which interleaves transaction commit messages into the source topic which are not propagated downstream.
*/
+@Tag("integration")
Review Comment:
Isn't this inherited already?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1938829826
@gharris1727 Would you be willing to add a brief comment stating the reason for the test being marked integration if it's one of "slow" or "flaky"? For the ones that make sense as integration tests more generally, we don't need to add a comment.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1938170491
> @dajac Maybe I misunderstand, but I don't see how the change you linked is relevant. If you run the tests once (as in locally running `./gradlew unitTest`) then the retries and merging of flaky tests in XML doesn't have an effect. And if you're using retries and merging the XML (as in Jenkins), you aren't using the unitTest target.
@gharris1727 Yeah, that's right. I was thinking about it from a CI point of view.
> A flaky test cannot be a unit test in my opinion. Unit tests are meant to be deterministic and fast. Anything that doesn't fit that definition should be tagged as integration test.
@ijuma I don't disagree. However, I think that they are tests that we should rather fix vs classifying them as integration tests.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1936821280
@mumrah I used this page: https://ge.apache.org/scans/tests?search.names=Git%20branch&search.relativeStartTime=P28D&search.rootProjectNames=kafka&search.timeZoneId=America%2FLos_Angeles&search.values=trunk&tests.sortField=FLAKY
and sorted by Flaky, Failed, and Mean Execution Time.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on code in PR #15349:
URL: https://github.com/apache/kafka/pull/15349#discussion_r1486595231
##########
connect/mirror/src/test/java/org/apache/kafka/connect/mirror/integration/MirrorConnectorsIntegrationTransactionsTest.java:
##########
@@ -31,6 +32,7 @@
* Integration test for MirrorMaker2 in which source records are emitted with a transactional producer,
* which interleaves transaction commit messages into the source topic which are not propagated downstream.
*/
+@Tag("integration")
Review Comment:
I changed this to make it consistent in the other direction, i removed the integration tag from all classes which should already inherit it.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1937051575
If we have some cases where a suite is mostly unit but it included some integration tests, it's ok to tag them all as integration and file a JIRA to fix it (move the integration test elsewhere and change the tagging).
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1937052374
One more thing: if it helps, we can add a comment explaining the reason for the tag so it's easier for future people to know.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1936910131
@dajac No, those were added because they were flaky: https://ge.apache.org/scans/tests?search.names=Git%20branch&search.relativeStartTime=P28D&search.rootProjectNames=kafka&search.timeZoneId=America%2FLos_Angeles&search.values=trunk&tests.container=org.apache.kafka.clients.admin.KafkaAdminClientTest&tests.sortField=FLAKY
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1938174110
Ah, I just saw the `Github build queue` thread. I better understand what you're trying to do here.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on code in PR #15349:
URL: https://github.com/apache/kafka/pull/15349#discussion_r1486493069
##########
connect/mirror/src/test/java/org/apache/kafka/connect/mirror/integration/MirrorConnectorsIntegrationTransactionsTest.java:
##########
@@ -31,6 +32,7 @@
* Integration test for MirrorMaker2 in which source records are emitted with a transactional producer,
* which interleaves transaction commit messages into the source topic which are not propagated downstream.
*/
+@Tag("integration")
Review Comment:
It is, so this change is actually a no-op, I didn't realize that.
It is more consistent with the other MM2 integration tests which also inherit the tag, but specify it anyway.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark tests with the most flaky failures as integration tests [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1936736732
Alright, I got 3 consecutive green builds, and I haven't seen any flaky failures locally.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on code in PR #15349:
URL: https://github.com/apache/kafka/pull/15349#discussion_r1485246193
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/HighAvailabilityTaskAssignorTest.java:
##########
@@ -97,6 +100,7 @@
import static org.mockito.Mockito.spy;
@RunWith(Parameterized.class)
+@Category({IntegrationTest.class})
Review Comment:
I looked into the 90 day history of this test, and the last failure was on Dec/11 the day we merged https://github.com/apache/kafka/pull/14965
Hence, this test should be stable now, and I don't think we need to add this tag, because it's really a unit test after all.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark tests with the most flaky failures as integration tests [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1936765040
This is very cool. Any chance you could do the same for the top N slowest unit tests (they're not unit tests if they're that slow)? Then the `unitTest` command would be useful enough to run locally before submitting PRs, etc.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on code in PR #15349:
URL: https://github.com/apache/kafka/pull/15349#discussion_r1486493069
##########
connect/mirror/src/test/java/org/apache/kafka/connect/mirror/integration/MirrorConnectorsIntegrationTransactionsTest.java:
##########
@@ -31,6 +32,7 @@
* Integration test for MirrorMaker2 in which source records are emitted with a transactional producer,
* which interleaves transaction commit messages into the source topic which are not propagated downstream.
*/
+@Tag("integration")
Review Comment:
It is, so this change is actually a no-op, I didn't realize that.
It is more consistent with the other MM2 integration tests which also inherit the tag.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1939299392
Hey all,
I realized thanks to @C0urante's review comment that the integration tag is already inherited from parent classes already, so the "added" tags were really no-ops. I've instead removed all of the duplicated integration tags where the class already inherits the tag from a parent class, including existing ones.
So when reviewing:
* If you see a tag removed, that should be a no-op change, as that test was already an integration test.
* If you see a tag added, that was a test which was previously unitTest, and is now integrationTest.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1995368843
@ijuma @mumrah @dajac @C0urante I'm still interested in having this change implemented, independent of hooking it into the CI build. PTAL, Thanks!
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1936811537
This is great, @gharris1727! How did you come up with the list of tests to mark as integration?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1936903652
@gharris1727 Thanks for looking into this. Overall, I like the idea. However, I see many suites that are unit tests and therefore we must be kept as unit tests (e.g. admin client test). Are those recategorized because there were slow?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "mjsax (via GitHub)" <gi...@apache.org>.
mjsax commented on code in PR #15349:
URL: https://github.com/apache/kafka/pull/15349#discussion_r1485247145
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamSlidingWindowAggregateTest.java:
##########
@@ -92,6 +94,7 @@
import static org.junit.Assert.assertTrue;
@RunWith(Parameterized.class)
+@Category({IntegrationTest.class})
Review Comment:
Looking into gradle enterprise, I cannot find any failing test run in the last 90 days for this test...
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "C0urante (via GitHub)" <gi...@apache.org>.
C0urante commented on code in PR #15349:
URL: https://github.com/apache/kafka/pull/15349#discussion_r1486653428
##########
connect/mirror/src/test/java/org/apache/kafka/connect/mirror/integration/MirrorConnectorsIntegrationTransactionsTest.java:
##########
@@ -31,6 +32,7 @@
* Integration test for MirrorMaker2 in which source records are emitted with a transactional producer,
* which interleaves transaction commit messages into the source topic which are not propagated downstream.
*/
+@Tag("integration")
Review Comment:
Good call, agree with removing redundant tags and reducing FUD 👍
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark tests with the most flaky failures as integration tests [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1936785675
We did save a little time, but not really too much because the build tends to spend a lot of time waiting for compileTestScala to complete :)
`./gradlew clean test` is 45 minutes on my machine,
`./gradlew clean unitTest` was ~10m, and now is ~8m.
If the compilation is already finished, `./gradlew unitTest` is ~5m.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1937051270
A flaky test cannot be a unit test in my opinion. Unit tests are meant to be deterministic and fast. Anything that doesn't fit that definition should be tagged as integration test.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on code in PR #15349:
URL: https://github.com/apache/kafka/pull/15349#discussion_r1485424799
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/assignment/HighAvailabilityTaskAssignorTest.java:
##########
@@ -97,6 +100,7 @@
import static org.mockito.Mockito.spy;
@RunWith(Parameterized.class)
+@Category({IntegrationTest.class})
Review Comment:
The average duration for this suite is ~4m30s, which is the 44th longest test suite in the project. This is borderline, but fell on the high side of the arbitrary line I chose.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on code in PR #15349:
URL: https://github.com/apache/kafka/pull/15349#discussion_r1485425218
##########
streams/src/test/java/org/apache/kafka/streams/kstream/internals/KStreamSlidingWindowAggregateTest.java:
##########
@@ -92,6 +94,7 @@
import static org.junit.Assert.assertTrue;
@RunWith(Parameterized.class)
+@Category({IntegrationTest.class})
Review Comment:
The average duration for this test is ~7m30s, the 24th longest test suite in the project.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #15349: KAFKA-16242: Mark slowest and most flaky tests as integration tests
URL: https://github.com/apache/kafka/pull/15349
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1938826340
> @ijuma I don't disagree. However, I think that they are tests that we should rather fix vs classifying them as integration tests.
As I said, they can be fixed - but they should be fixed separately versus blocking progress towards a useful `unitTest`.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-2300571340
This PR is being marked as stale since it has not had any activity in 90 days. If you would like to keep this PR alive, please ask a committer for review. If the PR has merge conflicts, please update it with the latest from trunk (or appropriate release branch) <p> If this PR is no longer valid or desired, please feel free to close it. If no activity occurs in the next 30 days, it will be automatically closed.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "dajac (via GitHub)" <gi...@apache.org>.
dajac commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1936915364
@gharris1727 I see. So if I understand correctly, two tests are flaky in the suite so we move the entire suite. This does not seem right to me. This particular suite is the one that you want to run when doing changes in the admin client.
I have been working on https://github.com/apache/kafka/pull/14862 which changes how we report flaky tests in Jenkins. I believe that this is a better was to handle them. What do you think?
However, I do agree with marking the slow tests as integration tests. I suppose that they should have been integration tests anyway.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "gharris1727 (via GitHub)" <gi...@apache.org>.
gharris1727 commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1936929467
@dajac Maybe I misunderstand, but I don't see how the change you linked is relevant. If you run the tests once (as in locally running `./gradlew unitTest`) then the retries and merging of flaky tests in XML doesn't have an effect. And if you're using retries and merging the XML (as in Jenkins), you aren't using the unitTest target.
But to your point: I agree that those tests are not integration tests according to the normal definitions, and this PR is effectively mis-using the terminology to instead mean:
* deterministic and fast: unit
* flaky or slow: integration
This division makes the unitTest target useful for reliable CI gating, something that we need urgently.
I suggested using a different tag for this, but others pushed back on this because the integration tag already does what we need, and another tag would have the same effect but a larger diff. But perhaps a `integration` tag, a `flaky` tag, and a `slow` tag could all be useful as in-line documentation as to why the test is demoted from unitTest.
I agree that these tests would be valuable to add to the unitTest argument, but I think that they should only be present once they are de-flaked. It is surprising to me that such foundational unit tests are flaky, and maybe that means that we should prioritize fixing them.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "C0urante (via GitHub)" <gi...@apache.org>.
C0urante commented on code in PR #15349:
URL: https://github.com/apache/kafka/pull/15349#discussion_r1485632038
##########
connect/mirror/src/test/java/org/apache/kafka/connect/mirror/integration/MirrorConnectorsIntegrationTransactionsTest.java:
##########
@@ -31,6 +32,7 @@
* Integration test for MirrorMaker2 in which source records are emitted with a transactional producer,
* which interleaves transaction commit messages into the source topic which are not propagated downstream.
*/
+@Tag("integration")
Review Comment:
Isn't this [inherited](https://github.com/apache/kafka/blob/d233eb98f7c7e55fe0dd673dbc058ddf619663a7/connect/mirror/src/test/java/org/apache/kafka/connect/mirror/integration/MirrorConnectorsIntegrationBaseTest.java#L103-L104) already?
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] KAFKA-16242: Mark slowest and most flaky tests as integration tests [kafka]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-2570032714
This PR has been closed since it has not had any activity in 120 days. If you feel like this
was a mistake, or you would like to continue working on it, please feel free to re-open the
PR and ask for a review.
--
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: jira-unsubscribe@kafka.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org