Refine GRADLE_RERUNNER #8522
Refine GRADLE_RERUNNER #8522
Conversation
blindpirate
added
from:member
a:chore
| <kotlin.version>1.3.0</kotlin.version> | ||
| <teamcity.dsl.version>2018.2</teamcity.dsl.version> | ||
| <mockk.version>1.9</mockk.version> | ||
| <junit.version>5.4.0</junit.version> |
There was a problem hiding this comment.
Switch to JUnit 5 because I don't see any reasons no doing so.
| </snapshots> | ||
| </repository> | ||
| </repositories> | ||
| <repositories> |
There was a problem hiding this comment.
Indents are adjusted because this is IDEA-default.
blindpirate
force-pushed
the
blindpirate/refine-rerunner
branch
from
b9551d8
to
9a58104
Compare
| @@ -221,7 +235,7 @@ class UnitTestAndCompilePlugin : Plugin<Project> { | |||
| fun Test.onlyRunPreviousFailedClassesIfNecessary() { | |||
| if (project.stringPropertyOrEmpty("onlyPreviousFailedTestClasses").toBoolean()) { | |||
| val previousFailedClasses = getPreviousFailedTestClasses() | |||
| if (previousFailedClasses.isEmpty()) { | |||
| if (previousFailedClasses.isEmpty() || allTestFailuresCount.get() > tooManyTestFailuresThreshold) { | |||
There was a problem hiding this comment.
This might be not accurate enough. For example, a build contains two test tasks, test task A has 8 failures, test task B has 6 failures, then both A and B should be disabled, but this logic will only disable test task B. I guess this is fine, we can adjust this logic and the threshold 10 if there's evidence they're not good enough.
|
|
||
| val buildScanTags = model.buildScanTags + listOfNotNull(buildType.stage?.id) | ||
| private | ||
| fun checkCleanM2Step(buildType: BaseGradleBuildType, os: OS = OS.linux) { |
There was a problem hiding this comment.
Shouldn't this be an extension function on BaseGradleBuildType?
Same goes for the other methods adding steps.
There was a problem hiding this comment.
Done. I was running into some issues previously when I tried to use extension function. Now I've fixed them.
|
|
||
| addKillProcessStep("KILL_PROCESSES_STARTED_BY_GRADLE") | ||
| private | ||
| fun gradleRerunnerStep(model: CIBuildModel, buildType: BaseGradleBuildType, gradleTasks: String, os: OS = OS.linux, extraParameters: String = "", daemon: Boolean = true) { |
There was a problem hiding this comment.
Do you think it makes sense to extract the common code and add a new flag (something like rerunFailedTest: Boolean instead of duplicating most of the logic?
There was a problem hiding this comment.
Personally I think current code is more clear to say "which build steps we have":
buildType.gradleRunnerStep(model, gradleTasks, os, extraParameters, daemon)
buildType.steps.extraSteps()
buildType.checkCleanM2Step(os)
buildType.verifyTestFilesCleanupStep(daemon)
buildType.tagBuildStep(model, daemon)
So I'd probably prefer this way.
| class UnitTestAndCompilePlugin : Plugin<Project> { | ||
| private | ||
| val allTestFailuresCount = AtomicInteger(0) |
There was a problem hiding this comment.
This is per project, isn't it? I think that is fine, though.
There was a problem hiding this comment.
Yes. Anyway we're running tests per project on CI, so that's fine.
| @@ -88,7 +89,18 @@ enum class ModuleType(val compatibility: JavaVersion) { | |||
| } | |||
|
|
|||
|
|
|||
| /** | |||
| * By default, we run an extra build step ("GRADLE_RERUNNER") which runs all test classes failed in the previous build step ("GRADLE_RUNNER"). | |||
| * However, if previous test failures are too many (>10), this is probably not caused by flainess. | |||
|
@wolfs PTAL |
|
Thanks @wolfs ! |
Context
This is the followup of #8470 : there're two issues unresolved:
GRADLE_RERUNNERis configured toGradleception/compileAll/sanityCheck, etc. , which makes no sense.GRADLE_RERUNNERif there're too many failures.This PR does:
applyTestDefaultsin addition toapplyDefaults. Functional test build and smoke test build applyapplyTestDefaults.applyDefaultsfunction.applyTestDefaults/applyDefaults..teamcityproject.