Prohibit JMH tests from having more than 2 forks #4075
Conversation
| List<TestDescriptor> failedTests = [] | ||
|
|
||
| test { | ||
| tasks.withType(Test) { |
There was a problem hiding this comment.
Reorganising tests: let's have a common set of config across all Test tasks
| failedTests.add(descriptor) | ||
| } | ||
| } | ||
|
|
| if (result.getFailedTestCount() > 0) { | ||
| failedTests.add(descriptor) | ||
| } | ||
| } |
|
|
||
| @Warmup(iterations = 2, time = 5, batchSize = 50) | ||
| @Measurement(iterations = 3, batchSize = 50) | ||
| @Fork(3) |
There was a problem hiding this comment.
Fix offending tests, even though these are in the benchmark package, and not performance (used by the automated pipeline)
| @@ -1,4 +1,4 @@ | |||
| package graphql | |||
| package graphql.archunit | |||
|
|
|||
There was a problem hiding this comment.
I'm going to write a few more ArchUnit tests, let's start organising them in 1 place
Test Results 324 files 324 suites 3m 26s ⏱️ Results for commit 697b463. |
There was a problem hiding this comment.
Pull Request Overview
This PR adds an ArchUnit test to enforce a maximum of 2 forks for JMH benchmarks to optimize CI pipeline performance, as additional forks provide diminishing returns in automated testing environments.
- Creates a new ArchUnit rule to validate JMH
@Forkannotations don't exceed value of 2 - Updates all existing JMH benchmark classes to use
@Fork(2)instead of@Fork(3) - Reorganizes ArchUnit tests into dedicated
graphql.archunitpackage for better organization
Reviewed Changes
Copilot reviewed 19 out of 20 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/test/groovy/graphql/archunit/NullabilityAnnotationUsageTest.groovy | Moved to new archunit package |
| src/test/groovy/graphql/archunit/JMHForkArchRuleTest.groovy | New ArchUnit test enforcing max 2 forks for JMH benchmarks |
| src/jmh/java/benchmark/*.java | Updated 17 benchmark classes to use @Fork(2) instead of @Fork(3) |
2 participants
Fixes #4010. Adds an ArchUnit test to prohibit JMH tests that use more than 2 forks. This is to save time in our automated testing pipeline, because extra forks don't add much value.
I had to update our test config because our JMH benchmarks are not in the usual package as everything else. ArchUnit reads .class files. Without this configuration change, ArchUnit can't read any JMH tests.
While I'm here I also:
performance), because I'd rather we apply this rule equally to all benchmarksSample error output