You are viewing a plain text version of this content. The canonical link for it is here.
Posted to jira@kafka.apache.org by "mumrah (via GitHub)" <gi...@apache.org> on 2025/01/31 17:53:47 UTC
[PR] (Draft) Load HEAD of test catalog for PRs [kafka]
mumrah opened a new pull request, #18770:
URL: https://github.com/apache/kafka/pull/18770
WIP
--
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] (Draft) Run new tests separately in PRs [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #18770:
URL: https://github.com/apache/kafka/pull/18770#issuecomment-2629984162
> For this same reason @RepeatedTest does not work with @ParameterizedTest.
Given that most use cases of @ParameterizedTest are now redundant since all integration tests are using Kraft mode, maybe we could file a PR to remove instances of ParameterizedTest + ValueSource(kraft) first. With that cleanup, we can leverage either `RepeatedTest` or custom template to run new integration tests multiple times
--
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-18748 Run new tests separately in PRs [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #18770:
URL: https://github.com/apache/kafka/pull/18770#issuecomment-2652511798
@chia7712 / @junrao any additional comments?
--
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-18748 Run new tests separately in PRs [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #18770:
URL: https://github.com/apache/kafka/pull/18770#issuecomment-2675376070
@chia7712 thanks for the review!
> What's the difference between new and auto-quarantined? I'm a bit confused.
These are the same concept. I have aligned on the term "new" since it is easier to type 😄
I also renamed AutoQuarantinedTestFilter to CatalogTestFilter and QuarantinedPostDiscoveryFilter to KafkaPostDiscoveryFilter
--
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-18748 Run new tests separately in PRs [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18770:
URL: https://github.com/apache/kafka/pull/18770#discussion_r1966585438
##########
build.gradle:
##########
@@ -578,45 +562,6 @@ subprojects {
finalizedBy("copyTestXml")
}
- task quarantinedTest(type: Test, dependsOn: compileJava) {
- // Disable caching and up-to-date for this task. We always want quarantined tests
- // to run and never want to cache their results.
- outputs.upToDateWhen { false }
- outputs.cacheIf { false }
-
- maxParallelForks = maxTestForks
- ignoreFailures = userIgnoreFailures
-
- maxHeapSize = defaultMaxHeapSize
- jvmArgs = defaultJvmArgs
-
- // KAFKA-17433 Used by deflake.yml github action to repeat individual tests
- systemProperty("kafka.cluster.test.repeat", project.findProperty("kafka.cluster.test.repeat"))
- systemProperty("kafka.test.catalog.file", project.findProperty("kafka.test.catalog.file"))
- systemProperty("kafka.test.run.quarantined", "true")
-
- testLogging {
- events = userTestLoggingEvents ?: testLoggingEvents
- showStandardStreams = userShowStandardStreams ?: testShowStandardStreams
- exceptionFormat = testExceptionFormat
- displayGranularity = 0
- }
- logTestStdout.rehydrate(delegate, owner, this)()
-
- useJUnitPlatform {
- includeEngines 'junit-jupiter'
- }
-
- develocity {
- testRetry {
- maxRetries = userMaxQuarantineTestRetries
Review Comment:
@mumrah any feedback about this comment? I assume both `maxQuarantineTestRetries` and `maxQuarantineTestRetryFailures` can be removed from `deflaky.yaml`. Both `userMaxQuarantineTestRetries` and `userMaxQuarantineTestRetryFailures` can be removed from `build.gradle.
--
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] (Draft) Run new tests separately in PRs [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #18770:
URL: https://github.com/apache/kafka/pull/18770#issuecomment-2629507278
Very nice - I think for the "new test suite" case, we also want to run the new tests multiple times. Perhaps 3 passes with no retries would be a reasonable bar?
--
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] (Draft) Run new tests separately in PRs [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #18770:
URL: https://github.com/apache/kafka/pull/18770#issuecomment-2631413304
@ijuma @chia7712: Looking more closely, I don't think we can repeat the unit tests within the JUnit framework. To repeat those tests, we would need to generate additional test invocation contexts which can only be done through a test template like `@RepeatableTest` or our own `@ClusterTest`. As far as I see it, the only way we could do this would be to introduce our own test annotation for unit tests which seems very disruptive and confusing.
An alternative would be to do the repetition at a higher level. For example, we could run the Gradle task in a loop or spawn parallel GHA jobs for the "new" tests.
Either way, I think that could be done separately from this PR. I will add the repetition for integration tests in this PR.
--
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-18748 Run new tests separately in PRs [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18770:
URL: https://github.com/apache/kafka/pull/18770#discussion_r1968426052
##########
.github/workflows/build.yml:
##########
@@ -210,45 +236,92 @@ jobs:
- name: Archive JUnit XML
uses: actions/upload-artifact@v4
with:
- name: junit-xml-${{ matrix.java }}
+ name: junit-xml-${{ env.job-variation }}
path: |
build/junit-xml/**/*.xml
compression-level: 9
if-no-files-found: ignore
- name: Archive Thread Dumps
- id: thread-dump-upload-artifact
- if: always() && (steps.junit-test.outputs.gradle-exitcode == '124' || steps.junit-quarantined-test.outputs.gradle-exitcode == '124')
+ id: archive-thread-dump
+ if: steps.junit-test.outputs.gradle-exitcode == '124'
uses: actions/upload-artifact@v4
with:
- name: junit-thread-dumps-${{ matrix.java }}
+ name: junit-thread-dumps-${{ env.job-variation }}
path: |
thread-dumps/*
compression-level: 9
if-no-files-found: ignore
- name: Parse JUnit tests
- run: python .github/scripts/junit.py --export-test-catalog ./test-catalog >> $GITHUB_STEP_SUMMARY
env:
GITHUB_WORKSPACE: ${{ github.workspace }}
- JUNIT_REPORT_URL: ${{ steps.junit-upload-artifact.outputs.artifact-url }}
- THREAD_DUMP_URL: ${{ steps.thread-dump-upload-artifact.outputs.artifact-url }}
+ JUNIT_REPORT_URL: ${{ steps.archive-junit-html.outputs.artifact-url }}
+ THREAD_DUMP_URL: ${{ steps.archive-thread-dump.outputs.artifact-url }}
GRADLE_TEST_EXIT_CODE: ${{ steps.junit-test.outputs.gradle-exitcode }}
- GRADLE_QUARANTINED_TEST_EXIT_CODE: ${{ steps.junit-quarantined-test.outputs.gradle-exitcode }}
+ run: |
+ python .github/scripts/junit.py \
+ --path build/junit-xml >> $GITHUB_STEP_SUMMARY
+ # This job downloads all the JUnit XML files and thread dumps from the JDK 23 test runs.
+ # If any test job fails, we will not run this job. Also, if any thread dump artifacts
+ # are present, this means there was a timeout in the tests and so we will not proceed
+ # with catalog creation.
+ collate-test-catalog:
+ name: Collate Test Catalog
+ needs: test
+ runs-on: ubuntu-latest
+ outputs:
+ uploaded-test-catalog: ${{ steps.archive-test-catalog.outcome == 'success' }}
+ steps:
+ - name: Checkout code
+ uses: actions/checkout@v4
+ with:
+ persist-credentials: false
+ - name: Download Thread Dumps
+ uses: actions/download-artifact@v4
+ with:
+ pattern: junit-thread-dumps-23-*
+ path: thread-dumps
+ merge-multiple: true
+ - name: Check For Thread Dump
+ id: check-for-thread-dump
+ run: |
+ find .
+ if [ -d thread-dumps ]; then
+ echo "Found 'thread-dumps' directory. Will not proceed with test catalog collation.";
+ exit 1;
+ fi
+ - name: Download JUnit XMLs
+ uses: actions/download-artifact@v4
+ with:
+ pattern: junit-xml-23-* # Only look at JDK 23 tests for the test catalog
+ path: junit-xml
+ merge-multiple: true
+ - name: Collate Test Catalog
+ continue-on-error: true
+ env:
+ GITHUB_WORKSPACE: ${{ github.workspace }}
+ GRADLE_TEST_EXIT_CODE: 0
+ run: |
+ python .github/scripts/junit.py \
Review Comment:
in this flow, `junit.py` prints the following message. Should it get fixed?
```
No reports available. Environment variable JUNIT_REPORT_URL was not found.
```
--
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] (Draft) Run new tests separately in PRs [kafka]
Posted by "junrao (via GitHub)" <gi...@apache.org>.
junrao commented on PR #18770:
URL: https://github.com/apache/kafka/pull/18770#issuecomment-2629585537
@mumrah : Thanks for the PR. Before, the test report has a section "Parse junit tests" that summarizes the test results. It seems gone in the new test results?
--
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-18748 Run new tests separately in PRs [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on code in PR #18770:
URL: https://github.com/apache/kafka/pull/18770#discussion_r1944962641
##########
test-common/test-common-util/src/main/java/org/apache/kafka/common/test/junit/QuarantinedPostDiscoveryFilter.java:
##########
@@ -41,42 +41,62 @@ public class QuarantinedPostDiscoveryFilter implements PostDiscoveryFilter {
public static final String RUN_QUARANTINED_PROP = "kafka.test.run.quarantined";
+ public static final String RUN_FLAKY_PROP = "kafka.test.run.flaky";
+
public static final String CATALOG_FILE_PROP = "kafka.test.catalog.file";
private final Filter<TestDescriptor> autoQuarantinedFilter;
+
private final boolean runQuarantined;
+ private final boolean runFlaky;
+
// No-arg public constructor for SPI
@SuppressWarnings("unused")
public QuarantinedPostDiscoveryFilter() {
runQuarantined = System.getProperty(RUN_QUARANTINED_PROP, "false")
.equalsIgnoreCase("true");
+ runFlaky = System.getProperty(RUN_FLAKY_PROP, "false")
+ .equalsIgnoreCase("true");
+
String testCatalogFileName = System.getProperty(CATALOG_FILE_PROP);
autoQuarantinedFilter = AutoQuarantinedTestFilter.create(testCatalogFileName, runQuarantined);
}
// Visible for tests
- QuarantinedPostDiscoveryFilter(Filter<TestDescriptor> autoQuarantinedFilter, boolean runQuarantined) {
+ QuarantinedPostDiscoveryFilter(
+ Filter<TestDescriptor> autoQuarantinedFilter,
+ boolean runQuarantined,
+ boolean runFlaky
+ ) {
this.autoQuarantinedFilter = autoQuarantinedFilter;
this.runQuarantined = runQuarantined;
+ this.runFlaky = runFlaky;
}
@Override
public FilterResult apply(TestDescriptor testDescriptor) {
boolean hasTag = testDescriptor.getTags().contains(FLAKY_TEST_TAG);
FilterResult result = autoQuarantinedFilter.apply(testDescriptor);
- if (runQuarantined) {
- // If selecting quarantined tests, we first check for explicitly flaky tests. If no
- // flaky tag is set, check the auto-quarantined filter. In the case of a missing test
- // catalog, the auto-quarantined filter will exclude all tests.
+
+ if (runFlaky && runQuarantined) {
Review Comment:
It is unreachable with our current configuration, but since it is possible to hit this I figured we should support it in code.
--
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] (Draft) Run new tests separately in PRs [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #18770:
URL: https://github.com/apache/kafka/pull/18770#issuecomment-2632790573
> I think that could be done separately from this PR
yes, that can be a follow-up
Additionally, could you please check the license of `AutoRepeatTestExtension.java` and please take a look at my previous questions :)
https://github.com/apache/kafka/pull/18770#discussion_r1938777525
https://github.com/apache/kafka/pull/18770#discussion_r1938769937
--
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-18748 Run new tests separately in PRs [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18770:
URL: https://github.com/apache/kafka/pull/18770#discussion_r1976251415
##########
README.md:
##########
@@ -42,7 +42,6 @@ Follow instructions in https://kafka.apache.org/quickstart
./gradlew test # runs both unit and integration tests
./gradlew unitTest
./gradlew integrationTest
- ./gradlew quarantinedTest # runs the quarantined tests
Review Comment:
#19025
--
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-18748 Run new tests separately in PRs [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #18770:
URL: https://github.com/apache/kafka/pull/18770#issuecomment-2679092657
@chia7712 I removed the remaining references to "quarantined" in build.gradle and aligned deflake.yml with our build.yml
--
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-18748 Run new tests separately in PRs [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on code in PR #18770:
URL: https://github.com/apache/kafka/pull/18770#discussion_r1966041807
##########
.github/scripts/thread-dump.sh:
##########
@@ -20,10 +20,12 @@ sleep $(($SLEEP_MINUTES*60));
echo "Timed out after $SLEEP_MINUTES minutes. Dumping threads now..."
mkdir thread-dumps
+touch thread-dumps/pids.txt
Review Comment:
Now that we are splitting the CI into parallel jobs, I wanted an easy way to test if one of them had a timeout. Rather than enumerating all the parallel jobs in an "if", I am downloading any thread dump artifacts and checking for this directory "thread-dumps"
```
- name: Download Thread Dumps
uses: actions/download-artifact@v4
with:
pattern: junit-thread-dumps-23-*
path: thread-dumps
merge-multiple: true
- name: Check For Thread Dump
id: check-for-thread-dump
run: |
find .
if [ -d thread-dumps ]; then
echo "Found 'thread-dumps' directory. Will not proceed with test catalog collation.";
exit 1;
fi
```
I place the "pids.txt" file there just to ensure the directory is included in the upload.
--
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-18748 Run new tests separately in PRs [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah merged PR #18770:
URL: https://github.com/apache/kafka/pull/18770
--
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-18748 Run new tests separately in PRs [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #18770:
URL: https://github.com/apache/kafka/pull/18770#issuecomment-2669850896
@chia7712 can you take another look at this one?
--
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-18748 Run new tests separately in PRs [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on code in PR #18770:
URL: https://github.com/apache/kafka/pull/18770#discussion_r1968457013
##########
.github/workflows/build.yml:
##########
@@ -210,45 +236,92 @@ jobs:
- name: Archive JUnit XML
uses: actions/upload-artifact@v4
with:
- name: junit-xml-${{ matrix.java }}
+ name: junit-xml-${{ env.job-variation }}
path: |
build/junit-xml/**/*.xml
compression-level: 9
if-no-files-found: ignore
- name: Archive Thread Dumps
- id: thread-dump-upload-artifact
- if: always() && (steps.junit-test.outputs.gradle-exitcode == '124' || steps.junit-quarantined-test.outputs.gradle-exitcode == '124')
+ id: archive-thread-dump
+ if: steps.junit-test.outputs.gradle-exitcode == '124'
uses: actions/upload-artifact@v4
with:
- name: junit-thread-dumps-${{ matrix.java }}
+ name: junit-thread-dumps-${{ env.job-variation }}
path: |
thread-dumps/*
compression-level: 9
if-no-files-found: ignore
- name: Parse JUnit tests
- run: python .github/scripts/junit.py --export-test-catalog ./test-catalog >> $GITHUB_STEP_SUMMARY
env:
GITHUB_WORKSPACE: ${{ github.workspace }}
- JUNIT_REPORT_URL: ${{ steps.junit-upload-artifact.outputs.artifact-url }}
- THREAD_DUMP_URL: ${{ steps.thread-dump-upload-artifact.outputs.artifact-url }}
+ JUNIT_REPORT_URL: ${{ steps.archive-junit-html.outputs.artifact-url }}
+ THREAD_DUMP_URL: ${{ steps.archive-thread-dump.outputs.artifact-url }}
GRADLE_TEST_EXIT_CODE: ${{ steps.junit-test.outputs.gradle-exitcode }}
- GRADLE_QUARANTINED_TEST_EXIT_CODE: ${{ steps.junit-quarantined-test.outputs.gradle-exitcode }}
+ run: |
+ python .github/scripts/junit.py \
+ --path build/junit-xml >> $GITHUB_STEP_SUMMARY
+ # This job downloads all the JUnit XML files and thread dumps from the JDK 23 test runs.
+ # If any test job fails, we will not run this job. Also, if any thread dump artifacts
+ # are present, this means there was a timeout in the tests and so we will not proceed
+ # with catalog creation.
+ collate-test-catalog:
+ name: Collate Test Catalog
+ needs: test
+ runs-on: ubuntu-latest
+ outputs:
+ uploaded-test-catalog: ${{ steps.archive-test-catalog.outcome == 'success' }}
+ steps:
+ - name: Checkout code
+ uses: actions/checkout@v4
+ with:
+ persist-credentials: false
+ - name: Download Thread Dumps
+ uses: actions/download-artifact@v4
+ with:
+ pattern: junit-thread-dumps-23-*
+ path: thread-dumps
+ merge-multiple: true
+ - name: Check For Thread Dump
+ id: check-for-thread-dump
+ run: |
+ find .
+ if [ -d thread-dumps ]; then
+ echo "Found 'thread-dumps' directory. Will not proceed with test catalog collation.";
+ exit 1;
+ fi
+ - name: Download JUnit XMLs
+ uses: actions/download-artifact@v4
+ with:
+ pattern: junit-xml-23-* # Only look at JDK 23 tests for the test catalog
+ path: junit-xml
+ merge-multiple: true
+ - name: Collate Test Catalog
+ continue-on-error: true
+ env:
+ GITHUB_WORKSPACE: ${{ github.workspace }}
+ GRADLE_TEST_EXIT_CODE: 0
+ run: |
+ python .github/scripts/junit.py \
Review Comment:
Ah, that is a side effect of running the junit.py script a second time to produce the test catalog. I doubt developers will be looking too much at the "Collate Test Catalog" step. I think it is okay to leave this.
--
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-18748 Run new tests separately in PRs [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on code in PR #18770:
URL: https://github.com/apache/kafka/pull/18770#discussion_r1968033161
##########
build.gradle:
##########
@@ -578,45 +562,6 @@ subprojects {
finalizedBy("copyTestXml")
}
- task quarantinedTest(type: Test, dependsOn: compileJava) {
- // Disable caching and up-to-date for this task. We always want quarantined tests
- // to run and never want to cache their results.
- outputs.upToDateWhen { false }
- outputs.cacheIf { false }
-
- maxParallelForks = maxTestForks
- ignoreFailures = userIgnoreFailures
-
- maxHeapSize = defaultMaxHeapSize
- jvmArgs = defaultJvmArgs
-
- // KAFKA-17433 Used by deflake.yml github action to repeat individual tests
- systemProperty("kafka.cluster.test.repeat", project.findProperty("kafka.cluster.test.repeat"))
- systemProperty("kafka.test.catalog.file", project.findProperty("kafka.test.catalog.file"))
- systemProperty("kafka.test.run.quarantined", "true")
-
- testLogging {
- events = userTestLoggingEvents ?: testLoggingEvents
- showStandardStreams = userShowStandardStreams ?: testShowStandardStreams
- exceptionFormat = testExceptionFormat
- displayGranularity = 0
- }
- logTestStdout.rehydrate(delegate, owner, this)()
-
- useJUnitPlatform {
- includeEngines 'junit-jupiter'
- }
-
- develocity {
- testRetry {
- maxRetries = userMaxQuarantineTestRetries
Review Comment:
Oh I totally missed this! Yes I'll remove these as well.
--
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] (Draft) Run new tests separately in PRs [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18770:
URL: https://github.com/apache/kafka/pull/18770#discussion_r1938769937
##########
.github/actions/run-gradle/action.yml:
##########
@@ -52,15 +67,21 @@ runs:
TIMEOUT_MINUTES: ${{ inputs.timeout-minutes}}
TEST_CATALOG: ${{ inputs.test-catalog-path }}
TEST_TASK: ${{ inputs.test-task }}
+ TEST_RETRIES: ${{ inputs.test-retries }}
+ RUN_NEW_TESTS: ${{ inputs.run-new-tests }}
+ RUN_FLAKY_TESTS: ${{ inputs.run-flaky-tests }}
+ TEST_XML_OUTPUT_DIR: ${{ inputs.test-xml-output }}
run: |
set +e
./.github/scripts/thread-dump.sh &
timeout ${TIMEOUT_MINUTES}m ./gradlew --build-cache --continue --no-scan \
-PtestLoggingEvents=started,passed,skipped,failed \
-PmaxParallelForks=2 \
- -PmaxTestRetries=1 -PmaxTestRetryFailures=3 \
- -PmaxQuarantineTestRetries=3 -PmaxQuarantineTestRetryFailures=0 \
+ -PmaxTestRetries=$TEST_RETRIES -PmaxTestRetryFailures=0 \
-Pkafka.test.catalog.file=$TEST_CATALOG \
+ -Pkafka.test.run.quarantined=$RUN_NEW_TESTS \
Review Comment:
Could we align the naming? otherwise, we may have some ambiguous terms in the future.
##########
test-common/test-common-util/src/main/java/org/apache/kafka/common/test/junit/QuarantinedPostDiscoveryFilter.java:
##########
@@ -41,42 +41,62 @@ public class QuarantinedPostDiscoveryFilter implements PostDiscoveryFilter {
public static final String RUN_QUARANTINED_PROP = "kafka.test.run.quarantined";
+ public static final String RUN_FLAKY_PROP = "kafka.test.run.flaky";
+
public static final String CATALOG_FILE_PROP = "kafka.test.catalog.file";
private final Filter<TestDescriptor> autoQuarantinedFilter;
+
private final boolean runQuarantined;
+ private final boolean runFlaky;
+
// No-arg public constructor for SPI
@SuppressWarnings("unused")
public QuarantinedPostDiscoveryFilter() {
runQuarantined = System.getProperty(RUN_QUARANTINED_PROP, "false")
.equalsIgnoreCase("true");
+ runFlaky = System.getProperty(RUN_FLAKY_PROP, "false")
+ .equalsIgnoreCase("true");
+
String testCatalogFileName = System.getProperty(CATALOG_FILE_PROP);
autoQuarantinedFilter = AutoQuarantinedTestFilter.create(testCatalogFileName, runQuarantined);
}
// Visible for tests
- QuarantinedPostDiscoveryFilter(Filter<TestDescriptor> autoQuarantinedFilter, boolean runQuarantined) {
+ QuarantinedPostDiscoveryFilter(
+ Filter<TestDescriptor> autoQuarantinedFilter,
+ boolean runQuarantined,
+ boolean runFlaky
+ ) {
this.autoQuarantinedFilter = autoQuarantinedFilter;
this.runQuarantined = runQuarantined;
+ this.runFlaky = runFlaky;
}
@Override
public FilterResult apply(TestDescriptor testDescriptor) {
boolean hasTag = testDescriptor.getTags().contains(FLAKY_TEST_TAG);
FilterResult result = autoQuarantinedFilter.apply(testDescriptor);
- if (runQuarantined) {
- // If selecting quarantined tests, we first check for explicitly flaky tests. If no
- // flaky tag is set, check the auto-quarantined filter. In the case of a missing test
- // catalog, the auto-quarantined filter will exclude all tests.
+
+ if (runFlaky && runQuarantined) {
Review Comment:
Pardon me, in the `build.yaml` we exclude the condition `run-flaky: true and run-new: true`, so is this a unreachable path?
--
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-18748 Run new tests separately in PRs [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on code in PR #18770:
URL: https://github.com/apache/kafka/pull/18770#discussion_r1966038212
##########
test-common/test-common-util/src/main/java/org/apache/kafka/common/test/junit/QuarantinedPostDiscoveryFilter.java:
##########
@@ -39,49 +41,101 @@ public class QuarantinedPostDiscoveryFilter implements PostDiscoveryFilter {
private static final TestTag FLAKY_TEST_TAG = TestTag.create("flaky");
- public static final String RUN_QUARANTINED_PROP = "kafka.test.run.quarantined";
+ public static final String RUN_NEW_PROP = "kafka.test.run.new";
+
+ public static final String RUN_FLAKY_PROP = "kafka.test.run.flaky";
public static final String CATALOG_FILE_PROP = "kafka.test.catalog.file";
+ public static final String VERBOSE_PROP = "kafka.test.verbose";
+
+ private static final Logger log = LoggerFactory.getLogger(QuarantinedPostDiscoveryFilter.class);
+
private final Filter<TestDescriptor> autoQuarantinedFilter;
- private final boolean runQuarantined;
+
+ private final boolean runNew;
+
+ private final boolean runFlaky;
+
+ private final boolean verbose;
// No-arg public constructor for SPI
@SuppressWarnings("unused")
public QuarantinedPostDiscoveryFilter() {
- runQuarantined = System.getProperty(RUN_QUARANTINED_PROP, "false")
+ runNew = System.getProperty(RUN_NEW_PROP, "false")
+ .equalsIgnoreCase("true");
+
+ runFlaky = System.getProperty(RUN_FLAKY_PROP, "false")
+ .equalsIgnoreCase("true");
+
+ verbose = System.getProperty(VERBOSE_PROP, "false")
.equalsIgnoreCase("true");
String testCatalogFileName = System.getProperty(CATALOG_FILE_PROP);
- autoQuarantinedFilter = AutoQuarantinedTestFilter.create(testCatalogFileName, runQuarantined);
+ autoQuarantinedFilter = AutoQuarantinedTestFilter.create(testCatalogFileName);
}
// Visible for tests
- QuarantinedPostDiscoveryFilter(Filter<TestDescriptor> autoQuarantinedFilter, boolean runQuarantined) {
+ QuarantinedPostDiscoveryFilter(
+ Filter<TestDescriptor> autoQuarantinedFilter,
+ boolean runNew,
+ boolean runFlaky
+ ) {
this.autoQuarantinedFilter = autoQuarantinedFilter;
- this.runQuarantined = runQuarantined;
+ this.runNew = runNew;
+ this.runFlaky = runFlaky;
+ this.verbose = false;
}
@Override
public FilterResult apply(TestDescriptor testDescriptor) {
- boolean hasTag = testDescriptor.getTags().contains(FLAKY_TEST_TAG);
- FilterResult result = autoQuarantinedFilter.apply(testDescriptor);
- if (runQuarantined) {
- // If selecting quarantined tests, we first check for explicitly flaky tests. If no
- // flaky tag is set, check the auto-quarantined filter. In the case of a missing test
- // catalog, the auto-quarantined filter will exclude all tests.
- if (hasTag) {
- return FilterResult.included("flaky");
+ boolean hasFlakyTag = testDescriptor.getTags().contains(FLAKY_TEST_TAG);
+ FilterResult autoQuarantinedResult = autoQuarantinedFilter.apply(testDescriptor);
+
+ final FilterResult result;
+ if (runFlaky && runNew) {
+ // If selecting flaky and quarantined tests, we first check for explicitly flaky tests.
+ // If no flaky tag is set, defer to the auto-quarantined filter.
+ if (hasFlakyTag) {
+ result = FilterResult.included("flaky");
+ } else {
+ result = autoQuarantinedResult;
+ }
+ } else if (runFlaky) {
+ // If selecting only flaky, just check the tag. Don't use the auto-quarantined filter
+ if (hasFlakyTag) {
+ result = FilterResult.included("flaky");
} else {
- return result;
+ result = FilterResult.excluded("non-flaky");
+ }
+ } else if (runNew) {
+ // Running only auto-quarantined
+ if (autoQuarantinedResult.included() && hasFlakyTag) {
+ result = FilterResult.excluded("flaky");
+ } else {
+ result = autoQuarantinedResult;
}
} else {
- // If selecting non-quarantined tests, we exclude auto-quarantined tests and flaky tests
- if (result.included() && hasTag) {
- return FilterResult.excluded("flaky");
+ // The main test suite
+ if (hasFlakyTag) {
+ result = FilterResult.excluded("flaky");
+ } else if (autoQuarantinedResult.included()) {
+ result = FilterResult.excluded("new");
} else {
- return result;
+ result = FilterResult.included(null);
}
}
+
+ if (verbose) {
+ log.info(
Review Comment:
I believe this uses Gradle's logger rather than Kafka's. I could be wrong though.
When I run
```
./gradlew --scan --info -Pkafka.test.run.flaky=true :metadata:cleanTest :metadata:test
```
I get the expected output on the command line
```
Gradle Test Executor 3 STANDARD_OUT
[2025-02-21 14:07:37,972] INFO Including Test 'JUnit Jupiter' with reason 'null'. Flaky tag is not present, auto-quarantined filter has not included this test. (org.apache.kafka.common.test.junit.QuarantinedPostDiscoveryFilter:130)
```
--
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] (Draft) Run new tests separately in PRs [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #18770:
URL: https://github.com/apache/kafka/pull/18770#issuecomment-2629488567
@junrao following up on our discussion from #18681. This shows how we could run new tests separately in PRs.
Here's a run of how it might look https://github.com/apache/kafka/actions/runs/13091901827/job/36529524359?pr=18770. If we use the matrix approach for this, we will end up running all six variations in parallel which will speed up PR builds just a bit.
cc @chia7712 @ijuma
--
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] (Draft) Run new tests separately in PRs [kafka]
Posted by "ijuma (via GitHub)" <gi...@apache.org>.
ijuma commented on PR #18770:
URL: https://github.com/apache/kafka/pull/18770#issuecomment-2629534407
Makes sense. I agree that unit tests should be generally stable and, if they're not, they should be considered 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] (Draft) Run new tests separately in PRs [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on PR #18770:
URL: https://github.com/apache/kafka/pull/18770#issuecomment-2630015893
open https://issues.apache.org/jira/browse/KAFKA-18695 to log the cleanup of `quorum=kraft`
--
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-18748 Run new tests separately in PRs [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18770:
URL: https://github.com/apache/kafka/pull/18770#discussion_r1968575877
##########
README.md:
##########
@@ -42,7 +42,6 @@ Follow instructions in https://kafka.apache.org/quickstart
./gradlew test # runs both unit and integration tests
./gradlew unitTest
./gradlew integrationTest
- ./gradlew quarantinedTest # runs the quarantined tests
Review Comment:
Should we use `./gradlew test -Pkafka.test.run.flaky=true` instead?
##########
README.md:
##########
@@ -77,10 +76,6 @@ The following example declares -PmaxTestRetries=1 and -PmaxTestRetryFailures=3 t
./gradlew test -PmaxTestRetries=1 -PmaxTestRetryFailures=3
-The quarantinedTest task also has no retries by default, but you can set maxQuarantineTestRetries and maxQuarantineTestRetryFailures to enable retries, similar to the test task.
-
- ./gradlew quarantinedTest -PmaxQuarantineTestRetries=3 -PmaxQuarantineTestRetryFailures=20
Review Comment:
ditto
--
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] (Draft) Run new tests separately in PRs [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #18770:
URL: https://github.com/apache/kafka/pull/18770#issuecomment-2629530430
@ijuma we could do this easily for any integration tests using `@ClusterTest` since we already have `kafka.cluster.test.repeat`. When I added that, I looked into doing it for regular unit tests but I don't think it will work. It boils down to JUnit only working with a single test template for a given test. For this same reason `@RepeatedTest` does not work with `@ParameterizedTest`.
However, maybe it would be sufficient to only do this for integration tests? Our pure unit tests tend to be fairly stable.
--
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-18748 Run new tests separately in PRs [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #18770:
URL: https://github.com/apache/kafka/pull/18770#issuecomment-2652494734
Added a fix for the build scan publish. Tested on my fork here https://github.com/mumrah/kafka/actions/runs/13273061929/job/37056775124. The publish step fails due to auth, but you can see that it successfully downloads the build scan from the CI job.
--
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] (Draft) Run new tests separately in PRs [kafka]
Posted by "mumrah (via GitHub)" <gi...@apache.org>.
mumrah commented on PR #18770:
URL: https://github.com/apache/kafka/pull/18770#issuecomment-2631350402
Thanks for taking a look everyone :)
@junrao, the junit.py parsing will be included. The script needs to be modified a bit to handle the different test directories. I wanted to float this idea before fully making the change. The output will be similar to how it looks today.
@chia7712 I'd like to avoid breaking compatibility with regular JUnit features like `@ParameterizedTest`. A cursory search shows a few usages beyond ZK/KRaft. With this change, PRs will have red builds if a single new test fails. Since PRs usually have a few builds along the way, maybe that gives us enough safety? At least it will be better than the current situation where a new test could fail 3 times and still get merged (if the author didn't check the test report)
--
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-18748 Run new tests separately in PRs [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18770:
URL: https://github.com/apache/kafka/pull/18770#discussion_r1965676176
##########
test-common/test-common-util/src/main/java/org/apache/kafka/common/test/junit/QuarantinedPostDiscoveryFilter.java:
##########
@@ -39,49 +41,101 @@ public class QuarantinedPostDiscoveryFilter implements PostDiscoveryFilter {
private static final TestTag FLAKY_TEST_TAG = TestTag.create("flaky");
- public static final String RUN_QUARANTINED_PROP = "kafka.test.run.quarantined";
+ public static final String RUN_NEW_PROP = "kafka.test.run.new";
+
+ public static final String RUN_FLAKY_PROP = "kafka.test.run.flaky";
public static final String CATALOG_FILE_PROP = "kafka.test.catalog.file";
+ public static final String VERBOSE_PROP = "kafka.test.verbose";
+
+ private static final Logger log = LoggerFactory.getLogger(QuarantinedPostDiscoveryFilter.class);
+
private final Filter<TestDescriptor> autoQuarantinedFilter;
- private final boolean runQuarantined;
+
+ private final boolean runNew;
+
+ private final boolean runFlaky;
+
+ private final boolean verbose;
// No-arg public constructor for SPI
@SuppressWarnings("unused")
public QuarantinedPostDiscoveryFilter() {
- runQuarantined = System.getProperty(RUN_QUARANTINED_PROP, "false")
+ runNew = System.getProperty(RUN_NEW_PROP, "false")
+ .equalsIgnoreCase("true");
+
+ runFlaky = System.getProperty(RUN_FLAKY_PROP, "false")
+ .equalsIgnoreCase("true");
+
+ verbose = System.getProperty(VERBOSE_PROP, "false")
.equalsIgnoreCase("true");
String testCatalogFileName = System.getProperty(CATALOG_FILE_PROP);
- autoQuarantinedFilter = AutoQuarantinedTestFilter.create(testCatalogFileName, runQuarantined);
+ autoQuarantinedFilter = AutoQuarantinedTestFilter.create(testCatalogFileName);
}
// Visible for tests
- QuarantinedPostDiscoveryFilter(Filter<TestDescriptor> autoQuarantinedFilter, boolean runQuarantined) {
+ QuarantinedPostDiscoveryFilter(
+ Filter<TestDescriptor> autoQuarantinedFilter,
+ boolean runNew,
+ boolean runFlaky
+ ) {
this.autoQuarantinedFilter = autoQuarantinedFilter;
- this.runQuarantined = runQuarantined;
+ this.runNew = runNew;
+ this.runFlaky = runFlaky;
+ this.verbose = false;
}
@Override
public FilterResult apply(TestDescriptor testDescriptor) {
- boolean hasTag = testDescriptor.getTags().contains(FLAKY_TEST_TAG);
- FilterResult result = autoQuarantinedFilter.apply(testDescriptor);
- if (runQuarantined) {
- // If selecting quarantined tests, we first check for explicitly flaky tests. If no
- // flaky tag is set, check the auto-quarantined filter. In the case of a missing test
- // catalog, the auto-quarantined filter will exclude all tests.
- if (hasTag) {
- return FilterResult.included("flaky");
+ boolean hasFlakyTag = testDescriptor.getTags().contains(FLAKY_TEST_TAG);
+ FilterResult autoQuarantinedResult = autoQuarantinedFilter.apply(testDescriptor);
+
+ final FilterResult result;
+ if (runFlaky && runNew) {
+ // If selecting flaky and quarantined tests, we first check for explicitly flaky tests.
+ // If no flaky tag is set, defer to the auto-quarantined filter.
+ if (hasFlakyTag) {
+ result = FilterResult.included("flaky");
+ } else {
+ result = autoQuarantinedResult;
+ }
+ } else if (runFlaky) {
+ // If selecting only flaky, just check the tag. Don't use the auto-quarantined filter
+ if (hasFlakyTag) {
+ result = FilterResult.included("flaky");
} else {
- return result;
+ result = FilterResult.excluded("non-flaky");
+ }
+ } else if (runNew) {
+ // Running only auto-quarantined
+ if (autoQuarantinedResult.included() && hasFlakyTag) {
+ result = FilterResult.excluded("flaky");
+ } else {
+ result = autoQuarantinedResult;
}
} else {
- // If selecting non-quarantined tests, we exclude auto-quarantined tests and flaky tests
- if (result.included() && hasTag) {
- return FilterResult.excluded("flaky");
+ // The main test suite
+ if (hasFlakyTag) {
+ result = FilterResult.excluded("flaky");
+ } else if (autoQuarantinedResult.included()) {
+ result = FilterResult.excluded("new");
} else {
- return result;
+ result = FilterResult.included(null);
}
}
+
+ if (verbose) {
+ log.info(
Review Comment:
does it work if `log4j2.yaml` does not declare info level?
##########
.github/scripts/thread-dump.sh:
##########
@@ -20,10 +20,12 @@ sleep $(($SLEEP_MINUTES*60));
echo "Timed out after $SLEEP_MINUTES minutes. Dumping threads now..."
mkdir thread-dumps
+touch thread-dumps/pids.txt
Review Comment:
Pardon me, what is the purpose of this file?
##########
build.gradle:
##########
@@ -578,45 +562,6 @@ subprojects {
finalizedBy("copyTestXml")
}
- task quarantinedTest(type: Test, dependsOn: compileJava) {
- // Disable caching and up-to-date for this task. We always want quarantined tests
- // to run and never want to cache their results.
- outputs.upToDateWhen { false }
- outputs.cacheIf { false }
-
- maxParallelForks = maxTestForks
- ignoreFailures = userIgnoreFailures
-
- maxHeapSize = defaultMaxHeapSize
- jvmArgs = defaultJvmArgs
-
- // KAFKA-17433 Used by deflake.yml github action to repeat individual tests
- systemProperty("kafka.cluster.test.repeat", project.findProperty("kafka.cluster.test.repeat"))
- systemProperty("kafka.test.catalog.file", project.findProperty("kafka.test.catalog.file"))
- systemProperty("kafka.test.run.quarantined", "true")
-
- testLogging {
- events = userTestLoggingEvents ?: testLoggingEvents
- showStandardStreams = userShowStandardStreams ?: testShowStandardStreams
- exceptionFormat = testExceptionFormat
- displayGranularity = 0
- }
- logTestStdout.rehydrate(delegate, owner, this)()
-
- useJUnitPlatform {
- includeEngines 'junit-jupiter'
- }
-
- develocity {
- testRetry {
- maxRetries = userMaxQuarantineTestRetries
Review Comment:
`userMaxQuarantineTestRetries` is removed, so we should not use `maxQuarantineTestRetries` in the CI flow, right?
##########
build.gradle:
##########
@@ -578,45 +562,6 @@ subprojects {
finalizedBy("copyTestXml")
}
- task quarantinedTest(type: Test, dependsOn: compileJava) {
- // Disable caching and up-to-date for this task. We always want quarantined tests
- // to run and never want to cache their results.
- outputs.upToDateWhen { false }
- outputs.cacheIf { false }
-
- maxParallelForks = maxTestForks
- ignoreFailures = userIgnoreFailures
-
- maxHeapSize = defaultMaxHeapSize
- jvmArgs = defaultJvmArgs
-
- // KAFKA-17433 Used by deflake.yml github action to repeat individual tests
- systemProperty("kafka.cluster.test.repeat", project.findProperty("kafka.cluster.test.repeat"))
- systemProperty("kafka.test.catalog.file", project.findProperty("kafka.test.catalog.file"))
- systemProperty("kafka.test.run.quarantined", "true")
-
- testLogging {
- events = userTestLoggingEvents ?: testLoggingEvents
- showStandardStreams = userShowStandardStreams ?: testShowStandardStreams
- exceptionFormat = testExceptionFormat
- displayGranularity = 0
- }
- logTestStdout.rehydrate(delegate, owner, this)()
-
- useJUnitPlatform {
- includeEngines 'junit-jupiter'
- }
-
- develocity {
- testRetry {
- maxRetries = userMaxQuarantineTestRetries
- maxFailures = userMaxQuarantineTestRetryFailures
Review Comment:
ditto
##########
test-common/test-common-util/src/main/java/org/apache/kafka/common/test/junit/AutoQuarantinedTestFilter.java:
##########
@@ -67,26 +67,10 @@ public FilterResult apply(TestDescriptor testDescriptor) {
MethodSource methodSource = (MethodSource) source;
TestAndMethod testAndMethod = new TestAndMethod(methodSource.getClassName(), methodSource.getMethodName());
- if (includeQuarantined) {
- if (testCatalog.contains(testAndMethod)) {
- return FilterResult.excluded("exclude non-quarantined");
- } else {
- return FilterResult.included("auto-quarantined");
- }
- } else {
- if (testCatalog.contains(testAndMethod)) {
- return FilterResult.included(null);
- } else {
- return FilterResult.excluded("auto-quarantined");
- }
- }
- }
-
- private static Filter<TestDescriptor> defaultFilter(boolean includeQuarantined) {
- if (includeQuarantined) {
- return EXCLUDE_ALL_TESTS;
+ if (testCatalog.contains(testAndMethod)) {
+ return FilterResult.excluded(null);
} else {
- return INCLUDE_ALL_TESTS;
+ return FilterResult.included("auto-quarantined");
Review Comment:
`QuarantinedPostDiscoveryFilter` uses the word `new` instead of `auto-quarantined`. should we align 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-18748 Run new tests separately in PRs [kafka]
Posted by "chia7712 (via GitHub)" <gi...@apache.org>.
chia7712 commented on code in PR #18770:
URL: https://github.com/apache/kafka/pull/18770#discussion_r1968583883
##########
README.md:
##########
@@ -42,7 +42,6 @@ Follow instructions in https://kafka.apache.org/quickstart
./gradlew test # runs both unit and integration tests
./gradlew unitTest
./gradlew integrationTest
- ./gradlew quarantinedTest # runs the quarantined tests
Review Comment:
I will file a minor patch for 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