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-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
