Skip to content

Commit f77702b

Browse files
Merge pull request #2697 from newrelic/FlakyUnitTests
Better handle flaky unit tests
2 parents 63ef7aa + 7c37355 commit f77702b

File tree

8 files changed

+77
-66
lines changed

8 files changed

+77
-66
lines changed

.github/workflows/GHA-Unit-Tests.yaml

Lines changed: 15 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,15 @@ on:
1818

1919
jobs:
2020
unit-test:
21-
name: Java ${{ matrix.java-version }}
21+
name: Java ${{ matrix.java-version }} - ${{ matrix.group }}
2222
timeout-minutes: 150
2323
runs-on: ubuntu-24.04
2424
strategy:
2525
# max-parallel: 1 ## used to force sequential vs. concurrent
2626
fail-fast: false
2727
matrix:
2828
java-version: [ 8, 11, 17, 21, 25 ]
29+
group: [flaky, nonForkedTests, forkedTests]
2930
steps:
3031
- name: Checkout Java agent
3132
uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # pin@v4
@@ -43,23 +44,23 @@ jobs:
4344
- name: Setup environment
4445
uses: ./.github/actions/setup-environment
4546

46-
- name: Run unit tests that do not require a forked JVM (attempt 1)
47+
- name: Run unit tests (attempt 1)
4748
id: run_tests_1
4849
continue-on-error: true
4950
timeout-minutes: 45
50-
run: ./gradlew $GRADLE_OPTIONS test -x :functional_test:test -x :newrelic-scala3-api:test -x :newrelic-scala-api:test -x :newrelic-scala-cats-api:test -x :newrelic-cats-effect3-api:test -x :newrelic-scala-monix-api:test -x :newrelic-scala-zio-api:test -x :newrelic-scala-zio2-api:test -Ptest${{ matrix.java-version }} -PnoInstrumentation -PnonForkedTests --continue
51+
run: ./gradlew $GRADLE_OPTIONS test -x :functional_test:test -x :newrelic-scala3-api:test -x :newrelic-scala-api:test -x :newrelic-scala-cats-api:test -x :newrelic-cats-effect3-api:test -x :newrelic-scala-monix-api:test -x :newrelic-scala-zio-api:test -x :newrelic-scala-zio2-api:test -Ptest${{ matrix.java-version }} -PnoInstrumentation -P${{ matrix.group }} --continue
5152

52-
- name: Run unit tests that do not require a forked JVM (attempt 2)
53+
- name: Run unit tests (attempt 2)
5354
id: run_tests_2
5455
continue-on-error: true
5556
timeout-minutes: 45
5657
if: steps.run_tests_1.outcome == 'failure'
57-
run: ./gradlew $GRADLE_OPTIONS test -x :functional_test:test -x :newrelic-scala3-api:test -x :newrelic-scala-api:test -x :newrelic-scala-cats-api:test -x :newrelic-cats-effect3-api:test -x :newrelic-scala-monix-api:test -x :newrelic-scala-zio-api:test -x :newrelic-scala-zio2-api:test -Ptest${{ matrix.java-version }} -PnoInstrumentation -PnonForkedTests --continue
58+
run: ./gradlew $GRADLE_OPTIONS test -x :functional_test:test -x :newrelic-scala3-api:test -x :newrelic-scala-api:test -x :newrelic-scala-cats-api:test -x :newrelic-cats-effect3-api:test -x :newrelic-scala-monix-api:test -x :newrelic-scala-zio-api:test -x :newrelic-scala-zio2-api:test -Ptest${{ matrix.java-version }} -PnoInstrumentation -P${{ matrix.group }} --continue
5859

59-
- name: Run unit tests that do not require a forked JVM (attempt 3)
60+
- name: Run unit tests (attempt 3)
6061
timeout-minutes: 45
6162
if: steps.run_tests_2.outcome == 'failure'
62-
run: ./gradlew $GRADLE_OPTIONS test -x :functional_test:test -x :newrelic-scala3-api:test -x :newrelic-scala-api:test -x :newrelic-scala-cats-api:test -x :newrelic-cats-effect3-api:test -x :newrelic-scala-monix-api:test -x :newrelic-scala-zio-api:test -x :newrelic-scala-zio2-api:test -Ptest${{ matrix.java-version }} -PnoInstrumentation -PnonForkedTests --continue
63+
run: ./gradlew $GRADLE_OPTIONS test -x :functional_test:test -x :newrelic-scala3-api:test -x :newrelic-scala-api:test -x :newrelic-scala-cats-api:test -x :newrelic-cats-effect3-api:test -x :newrelic-scala-monix-api:test -x :newrelic-scala-zio-api:test -x :newrelic-scala-zio2-api:test -Ptest${{ matrix.java-version }} -PnoInstrumentation -P${{ matrix.group }} --continue
6364

6465
- name: Upload coverage to Codecov
6566
if: matrix.java-version == '17'
@@ -73,63 +74,16 @@ jobs:
7374
if: matrix.java-version == '17'
7475
uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # pin@v4
7576
with:
76-
name: non-forked-jacoco-reports-java-${{ matrix.java-version }}
77+
name: ${{ matrix.group }}-jacoco-reports-java-${{ matrix.java-version }}
7778
path: |
7879
**/build/reports/jacoco/**
7980
80-
- name: Capture build reports (non forked)
81+
- name: Capture build reports
8182
# If previous step fails, run this step regardless
8283
if: failure()
8384
uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # pin@v4
8485
with:
85-
name: non-forked-tests-results-java-${{ matrix.java-version }}
86-
# The regex for the path below will capture unit test HTML reports generated by gradle for all
87-
# related modules: (agent-bridge, newrelic-java, newrelic-api, etc).
88-
# However, it's critical that the previous build step does a ./gradlew clean or the regex will capture test reports
89-
# that were leftover in unrelated modules for functional and instrumentation tests.
90-
path: |
91-
**/build/reports/tests/*
92-
93-
- name: Run unit tests that require a forked JVM (attempt 1)
94-
id: run_forked_tests_1
95-
continue-on-error: true
96-
timeout-minutes: 45
97-
run: ./gradlew $GRADLE_OPTIONS test -x :functional_test:test -x :newrelic-scala3-api:test -x :newrelic-scala-api:test -x :newrelic-scala-cats-api:test -x :newrelic-cats-effect3-api:test -x :newrelic-scala-monix-api:test -x :newrelic-scala-zio-api:test -x :newrelic-scala-zio2-api:test -Ptest${{ matrix.java-version }} -PnoInstrumentation -PforkedTests --continue
98-
99-
- name: Run unit tests that require a forked JVM (attempt 2)
100-
id: run_forked_tests_2
101-
continue-on-error: true
102-
timeout-minutes: 45
103-
if: steps.run_forked_tests_1.outcome == 'failure'
104-
run: ./gradlew $GRADLE_OPTIONS test -x :functional_test:test -x :newrelic-scala3-api:test -x :newrelic-scala-api:test -x :newrelic-scala-cats-api:test -x :newrelic-cats-effect3-api:test -x :newrelic-scala-monix-api:test -x :newrelic-scala-zio-api:test -x :newrelic-scala-zio2-api:test -Ptest${{ matrix.java-version }} -PnoInstrumentation -PforkedTests --continue
105-
106-
- name: Run unit tests that require a forked JVM (attempt 3)
107-
id: run_forked_tests_3
108-
timeout-minutes: 45
109-
if: steps.run_forked_tests_2.outcome == 'failure'
110-
run: ./gradlew $GRADLE_OPTIONS test -x :functional_test:test -x :newrelic-scala3-api:test -x :newrelic-scala-api:test -x :newrelic-scala-cats-api:test -x :newrelic-cats-effect3-api:test -x :newrelic-scala-monix-api:test -x :newrelic-scala-zio-api:test -x :newrelic-scala-zio2-api:test -Ptest${{ matrix.java-version }} -PnoInstrumentation -PforkedTests --continue
111-
112-
- name: Upload coverage to Codecov
113-
if: matrix.java-version == '17'
114-
uses: codecov/codecov-action@6d798873df2b1b8e5846dba6fb86631229fbcb17 # pin@v4
115-
with:
116-
token: ${{ secrets.CODECOV_TOKEN }}
117-
files: '**/build/reports/jacoco/test/jacocoTestReport.xml'
118-
fail_ci_if_error: false #default is false, but being explicit about what to expect.
119-
120-
- name: Capture Jacoco reports
121-
if: matrix.java-version == '17'
122-
uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # pin@v4
123-
with:
124-
name: forked-jacoco-reports-java-${{ matrix.java-version }}
125-
path: |
126-
**/build/reports/jacoco/**
127-
128-
- name: Capture build reports (forked)
129-
if: failure()
130-
uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # pin@v4
131-
with:
132-
name: forked-tests-results-java-${{ matrix.java-version }}
86+
name: ${{ matrix.group }}-tests-results-java-${{ matrix.java-version }}
13387
# The regex for the path below will capture unit test HTML reports generated by gradle for all
13488
# related modules: (agent-bridge, newrelic-java, newrelic-api, etc).
13589
# However, it's critical that the previous build step does a ./gradlew clean or the regex will capture test reports
@@ -150,12 +104,12 @@ jobs:
150104
git config --global user.name "GitHub Actions Bot"
151105
git config --global user.email "[email protected]"
152106
153-
- name: Download test reports Java ${{ matrix.java-version }}
107+
- name: Download test reports Java ${{ matrix.java-version }} - ${{ matrix.group }}
154108
if: ${{ failure() }}
155109
uses: actions/download-artifact@65a9edc5881444af0b9093a5e628f2fe47ea3b2e # pin@v4
156110
with:
157-
name: unit-tests-results-java-${{ matrix.java-version }}
158-
path: gh-pages/reports/${{ inputs.agent-ref }}/${{ github.run_id }}/unit-tests-results-java-${{ matrix.java-version }}
111+
name: unit-tests-results-java-${{ matrix.java-version }}-${{ matrix.group }}
112+
path: gh-pages/reports/${{ inputs.agent-ref }}/${{ github.run_id }}/unit-tests-results-java-${{ matrix.java-version }}-${{ matrix.group }}
159113

160114
- name: Set up Python
161115
if: ${{ failure() }}
@@ -168,7 +122,7 @@ jobs:
168122
if: ${{ failure() }}
169123
run: |
170124
pip install beautifulsoup4
171-
python gh-pages/utils/writeTestsFailuresPage.py gh-pages/reports/${{ inputs.agent-ref }}/${{ github.run_id }}/unit-tests-results-java-${{ matrix.java-version }}
125+
python gh-pages/utils/writeTestsFailuresPage.py gh-pages/reports/${{ inputs.agent-ref }}/${{ github.run_id }}/unit-tests-results-java-${{ matrix.java-version }}-${{ matrix.group }}
172126
173127
- name: Commit and push test reports to gh-pages
174128
if: ${{ failure() }}

newrelic-agent/build.gradle

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -326,11 +326,13 @@ test {
326326
forkEvery = 1
327327
maxParallelForks = Runtime.runtime.availableProcessors()
328328
useJUnit {
329-
if (project.hasProperty("forkedTests")) {
329+
if (project.hasProperty("flaky")) {
330+
includeCategories("com.newrelic.test.marker.Flaky")
331+
} else if (project.hasProperty("forkedTests")) {
330332
includeCategories("com.newrelic.test.marker.RequiresFork")
331333
} else if (project.hasProperty("nonForkedTests")) {
332334
forkEvery = 10
333-
excludeCategories("com.newrelic.test.marker.RequiresFork")
335+
excludeCategories("com.newrelic.test.marker.RequiresFork", "com.newrelic.test.marker.Flaky")
334336
}
335337
}
336338

newrelic-agent/src/test/java/com/newrelic/agent/jfr/JfrServiceTest.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import com.newrelic.jfr.ThreadNameNormalizer;
1414
import com.newrelic.jfr.daemon.DaemonConfig;
1515
import com.newrelic.jfr.daemon.JfrRecorderException;
16+
import com.newrelic.test.marker.Flaky;
1617
import com.newrelic.test.marker.IBMJ9IncompatibleTest;
1718
import org.junit.Before;
1819
import org.junit.Test;
@@ -126,8 +127,9 @@ public void jfrLoopDoesNotStartWhenIsEnabledIsTrueAndHighSecurityIsTrue() throws
126127
verify(spyJfr, times(0)).startJfrLoop();
127128
}
128129

129-
@Category( IBMJ9IncompatibleTest.class )
130+
@Category( { IBMJ9IncompatibleTest.class, Flaky.class } )
130131
@Test
132+
// Flaky note: org.mockito.exceptions.verification.WantedButNotInvoked on verify(spyJfr, timeout(100)).startJfrLoop();
131133
public void jfrLoopDoesStart() {
132134
JfrService jfrService = new JfrService(jfrConfig, agentConfig);
133135
JfrService spyJfr = spy(jfrService);

newrelic-agent/src/test/java/com/newrelic/agent/service/module/JarCollectorServiceProcessorTest.java

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,9 @@
1111
import com.newrelic.agent.config.AgentConfig;
1212
import com.newrelic.agent.config.JarCollectorConfig;
1313
import com.newrelic.api.agent.Logger;
14+
import com.newrelic.test.marker.Flaky;
1415
import org.junit.Test;
16+
import org.junit.experimental.categories.Category;
1517
import org.mockito.ArgumentMatchers;
1618

1719
import javax.servlet.jsp.JspPage;
@@ -94,6 +96,11 @@ private AgentConfig getMockConfig() {
9496
}
9597

9698
@Test
99+
@Category( Flaky.class )
100+
// Flaky note: Somehow the elapsedMillis is sometimes < 4000, which probably shouldn't happen, but
101+
// I haven't investigated the warmup time of the SmoothBursty RateLimiter class
102+
// Also, I put this in a loop and ran locally 1000 times and never saw a failure, so it
103+
// may be another strange timing issue in GH.
97104
public void applyWithRateLimit() throws URISyntaxException {
98105
AgentConfig config = getMockConfig();
99106
when(config.getJarCollectorConfig().getJarsPerSecond()).thenReturn(10);

newrelic-agent/src/test/java/com/newrelic/agent/tracing/samplers/AdaptiveSamplerTest.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,11 @@
22

33
import com.newrelic.agent.MockServiceManager;
44
import com.newrelic.agent.tracing.DistributedTraceUtil;
5+
import com.newrelic.test.marker.Flaky;
56
import org.junit.Assert;
67
import org.junit.Before;
78
import org.junit.Test;
9+
import org.junit.experimental.categories.Category;
810

911
import java.util.List;
1012
import java.util.ArrayList;
@@ -170,6 +172,8 @@ public void testExponentialBackoff() throws InterruptedException, ExecutionExcep
170172
}
171173

172174
@Test
175+
@Category( Flaky.class )
176+
// Flaky note: expectedSampled ends up being 15 and 16s and 17s are fairly common
173177
public void testCalculatePriorityMultithreaded() throws InterruptedException {
174178
int target = 10;
175179
int reportPeriod = 5;
@@ -243,7 +247,7 @@ public int runSamplerConcurrentAndGetSampled(AdaptiveSampler sampler, long total
243247
AtomicInteger totalSampled = new AtomicInteger(0);
244248
//in this example, the wait is random, and the load is distributed across 4 threads.
245249
Random random = new Random();
246-
int MAX_WAIT_BETWEEN_SAMPLES = 500;
250+
int MAX_WAIT_BETWEEN_SAMPLES = 50;
247251
//Start it up
248252
long startTime = System.currentTimeMillis();
249253
for (int i = 0; i < nThreads ; i++) {

newrelic-agent/src/test/java/com/newrelic/agent/transport/DataSenderImplTest.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,14 @@
3131
import com.newrelic.agent.stats.RecordDataUsageMetric;
3232
import com.newrelic.agent.stats.StatsImpl;
3333
import com.newrelic.agent.stats.StatsService;
34+
import com.newrelic.test.marker.Flaky;
3435
import org.hamcrest.CoreMatchers;
3536
import org.junit.After;
3637
import org.junit.Assert;
3738
import org.junit.Before;
3839
import org.junit.Rule;
3940
import org.junit.Test;
41+
import org.junit.experimental.categories.Category;
4042
import org.junit.rules.ExpectedException;
4143
import org.mockito.ArgumentMatchers;
4244
import org.mockito.Mock;
@@ -243,6 +245,9 @@ public void dataReceived(String method, String encoding, String uri, Map<?, ?> r
243245
}
244246

245247
@Test
248+
@Category( Flaky.class )
249+
// Flaky note: java.lang.ArrayIndexOutOfBoundsException inside assertDataUsageMetricValues
250+
// on Object rawArgument = invocation.getRawArguments()[0];
246251
public void testDataUsageSupportability() throws Exception {
247252
AgentConfig config = AgentConfigImpl.createAgentConfig(configMap());
248253
HttpClientWrapper wrapperEmptyReturn = getHttpClientWrapper(ReadResult.create(

newrelic-agent/src/test/java/com/newrelic/bootstrap/EmbeddedJarFilesImplTest.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
package com.newrelic.bootstrap;
22

3+
import com.newrelic.test.marker.Flaky;
34
import org.junit.After;
45
import org.junit.Assert;
56
import org.junit.Before;
67
import org.junit.Ignore;
78
import org.junit.Rule;
89
import org.junit.Test;
10+
import org.junit.experimental.categories.Category;
911
import org.junit.rules.TemporaryFolder;
1012

1113
import java.io.File;
@@ -102,6 +104,9 @@ public void cleanupStaleTempJarFiles_onlyDeletesJarFiles() throws Exception {
102104
}
103105

104106
@Test
107+
@Category( Flaky.class )
108+
// Flaky note: GHA can sometimes take a very long time to go from afterThresholdJar.setLastModified to
109+
// setting the cutoff time in the Impl class, which leads to the file being erroneously deleted
105110
public void cleanupStaleTempJarFiles_respectsThresholdExactly() throws Exception {
106111
long now = System.currentTimeMillis();
107112
long fiveHoursInMillis = 5 * 60 * 60 * 1000L;
@@ -112,7 +117,7 @@ public void cleanupStaleTempJarFiles_respectsThresholdExactly() throws Exception
112117

113118
// Create jar just after threshold - should not be deleted
114119
File afterThresholdJar = createTempJarFile(BootstrapLoader.AGENT_BRIDGE_JAR_NAME + "33333.jar", 0);
115-
afterThresholdJar.setLastModified(now - fiveHoursInMillis + 1);
120+
afterThresholdJar.setLastModified(now - fiveHoursInMillis + 15000); // large padding for GHA warmup
116121

117122
setThresholdAndCleanup(5);
118123

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/*
2+
*
3+
* * Copyright 2026 New Relic Corporation. All rights reserved.
4+
* * SPDX-License-Identifier: Apache-2.0
5+
*
6+
*/
7+
8+
package com.newrelic.test.marker;
9+
10+
/**
11+
* <p>
12+
* Marker interface to denote a unit/functional/instrumentation test has proven flaky.
13+
* This allows us to categorize these and more easily re-run them without having to
14+
* re-run the entire test suite.
15+
* </p>
16+
* <p>
17+
* This category currently only affects tests in newrelic-agent when the tests are run with
18+
* <code>-Pflaky</code>.
19+
* </p>
20+
* <p>
21+
* <b>Note: This annotation will override RequiresFork.</b>
22+
* </p>
23+
*
24+
* <p>
25+
* To mark your test with this category, add the following annotation to the class.
26+
* <pre>
27+
* {@code @Category({ Flaky.class })}
28+
* </pre>
29+
* </p>
30+
*/
31+
public interface Flaky {
32+
}

0 commit comments

Comments
 (0)