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
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
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.
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
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
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
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
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
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,
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
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
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
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
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
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
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
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.
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
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.
--
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
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
gharris1727 commented on PR #15349:
URL: https://github.com/apache/kafka/pull/15349#issuecomment-1936910131
@dajac No, those were added because they were flaky:
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).
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=P28D=kafka=America%2FLos_Angeles=trunk=FLAKY
and sorted by Flaky, Failed, and Mean Execution
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,
25 matches
Mail list logo
