From cb29c966c870bcd17996fe49fed1fff64ca7aefc Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Thu, 2 Jan 2025 12:45:23 +0100 Subject: [PATCH] Improve debuggability of failing profiler tests --- .github/workflows/test_workflow.yml | 70 +++++++++++++++---- README.md | 10 +++ ddprof-lib/gtest/build.gradle | 8 +-- ddprof-test/build.gradle | 4 +- .../profiler/AbstractProfilerTest.java | 12 +++- 5 files changed, 81 insertions(+), 23 deletions(-) diff --git a/.github/workflows/test_workflow.yml b/.github/workflows/test_workflow.yml index 4d49be4b..90966905 100644 --- a/.github/workflows/test_workflow.yml +++ b/.github/workflows/test_workflow.yml @@ -83,7 +83,7 @@ jobs: printf "%s.%s.%s\n", v[1], v[2], v[3] } }') - ./gradlew :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs + ./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs ls -la .gradle if [ $? -ne 0 ]; then echo "glibc-${{ matrix.java_version }}-${{ matrix.config }}-amd64" >> failures_glibc-${{ matrix.java_version }}-${{ matrix.config }}-amd64.txt @@ -110,6 +110,11 @@ jobs: with: name: failures path: failures_glibc-${{ matrix.java_version }}-${{ matrix.config }}-amd64.txt + - uses: actions/upload-artifact@v3 + if: failure() + with: + name: recordings + path: /tmp/*.jfr test-linux-glibc-aarch64: strategy: @@ -176,7 +181,7 @@ jobs: export LIBC=glibc export JAVA_TEST_HOME=$(pwd)/test_${{ env.JAVA_VERSION }}_jdk export SANITIZER=${{ matrix.config }} - ./gradlew :ddprof-test:test${{ matrix.config }} + ./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs if [ $? -ne 0 ]; then echo "glibc-${{ matrix.java_version }}-${{ matrix.config }}-aarch64" >> failures_glibc-${{ matrix.java_version }}-${{ matrix.config }}-aarch64.txt exit 1 @@ -202,6 +207,11 @@ jobs: with: name: failures path: failures_glibc-${{ matrix.java_version }}-${{ matrix.config }}-aarch64.txt + - uses: actions/upload-artifact@v3 + if: failure() + with: + name: recordings + path: /tmp/*.jfr test-linux-glibc-j9-amd64: strategy: @@ -248,7 +258,7 @@ jobs: run: | set +e export TEST_COMMIT=${{ github.sha }} - export TEST_CONFIGURATION=glibc/${{ matrix.java_version }} + export TEST_CONFIGURATION=glibc/j9/${{ matrix.java_version }}-amd64 export LIBC=glibc export SANITIZER=${{ matrix.config }} export JAVA_VERSION=$(${JAVA_TEST_HOME}/bin/java -version 2>&1 | awk -F '"' '/version/ { @@ -262,7 +272,7 @@ jobs: } }') chmod a+x gradlew - ./gradlew :ddprof-test:test${{ matrix.config }} + ./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs if [ $? -ne 0 ]; then echo "glibc-j9-${{ matrix.java_version }}-${{ matrix.config }}-amd64" >> failures_glibc-j9-${{ matrix.java_version }}-${{ matrix.config }}-amd64.txt exit 1 @@ -288,6 +298,11 @@ jobs: with: name: failures path: failures_glibc-j9-${{ matrix.java_version }}-${{ matrix.config }}-amd64.txt + - uses: actions/upload-artifact@v3 + if: failure() + with: + name: recordings + path: /tmp/*.jfr test-linux-glibc-j9-aarch64: strategy: @@ -337,7 +352,7 @@ jobs: run: | set +e export TEST_COMMIT=${{ github.sha }} - export TEST_CONFIGURATION=glibc/${{ matrix.java_version }} + export TEST_CONFIGURATION=glibc/j9/${{ matrix.java_version }}-aarch64 export LIBC=glibc export SANITIZER=${{ matrix.config }} export JAVA_VERSION=$(${JAVA_TEST_HOME}/bin/java -version 2>&1 | awk -F '"' '/version/ { @@ -351,7 +366,7 @@ jobs: } }') chmod a+x gradlew - ./gradlew :ddprof-test:test${{ matrix.config }} + ./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs if [ $? -ne 0 ]; then echo "glibc-j9-${{ matrix.java_version }}-${{ matrix.config }}-aarch64" >> failures_glibc-j9-${{ matrix.java_version }}-${{ matrix.config }}-aarch64.txt exit 1 @@ -377,6 +392,11 @@ jobs: with: name: failures path: failures_glibc-j9-${{ matrix.java_version }}-${{ matrix.config }}-aarch64.txt + - uses: actions/upload-artifact@v3 + if: failure() + with: + name: recordings + path: /tmp/*.jfr test-linux-glibc-oracle8: strategy: @@ -433,7 +453,7 @@ jobs: run: | set +e export TEST_COMMIT=${{ github.sha }} - export TEST_CONFIGURATION=glibc/8 + export TEST_CONFIGURATION=glibc/oracle/8-amd64 export LIBC=glibc export JAVA_TEST_HOME=/usr/lib/jvm/oracle8 export JAVA_HOME=$JAVA_HOME @@ -449,7 +469,7 @@ jobs: printf "%s.%s.%s\n", v[1], v[2], v[3] } }') - ./gradlew :ddprof-test:test${{ matrix.config }} + ./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs if [ $? -ne 0 ]; then echo "glibc-oracle8-${{ matrix.config }}" >> failures_glibc-oracle8-${{ matrix.config }}.txt exit 1 @@ -474,6 +494,11 @@ jobs: with: name: failures path: failures_glibc-oracle8-${{ matrix.config }}.txt + - uses: actions/upload-artifact@v3 + if: failure() + with: + name: recordings + path: /tmp/*.jfr test-linux-musl-amd64: strategy: @@ -546,7 +571,7 @@ jobs: export JAVA_HOME=$(pwd)/test_11.0.25_jdk export PATH=$JAVA_HOME/bin:$PATH export TEST_COMMIT=${{ github.sha }} - export TEST_CONFIGURATION=musl/${{ matrix.java_version }} + export TEST_CONFIGURATION=musl/${{ matrix.java_version }}-amd64 export LIBC=musl export JAVA_TEST_HOME=$(pwd)/test_${{ env.JAVA_VERSION }}_jdk export SANITIZER=${{ matrix.config }} @@ -560,7 +585,7 @@ jobs: printf "%s.%s.%s\n", v[1], v[2], v[3] } }') - ./gradlew :ddprof-test:test${{ matrix.config }} + ./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs if [ $? -ne 0 ]; then echo "musl-${{ matrix.java_version }}-${{ matrix.config }}-amd64" >> failures_musl-${{ matrix.java_version }}-${{ matrix.config }}-amd64.txt exit 1 @@ -586,6 +611,11 @@ jobs: with: name: failures path: failures_musl-${{ matrix.java_version }}-${{ matrix.config }}-amd64.txt + - uses: actions/upload-artifact@v3 + if: failure() + with: + name: recordings + path: /tmp/*.jfr test-linux-glibc-zing-amd64: strategy: @@ -665,7 +695,7 @@ jobs: run: | set +e export TEST_COMMIT=${{ github.sha }} - export TEST_CONFIGURATION=glibc/${{ matrix.java_version }} + export TEST_CONFIGURATION=glibc/zing/${{ matrix.java_version }}-amd64 export LIBC=glibc export JAVA_TEST_HOME=$(pwd)/test_${{ matrix.java_version }}_jdk export JAVA_HOME=$JAVA_HOME @@ -681,7 +711,7 @@ jobs: printf "%s.%s.%s\n", v[1], v[2], v[3] } }') - ./gradlew :ddprof-test:test${{ matrix.config }} + ./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs if [ $? -ne 0 ]; then echo "glibc-zing-${{ matrix.java_version }}-${{ matrix.config }}-amd64" >> failures_zing-${{ matrix.java_version }}-${{ matrix.config }}-amd64.txt exit 1 @@ -706,6 +736,11 @@ jobs: with: name: failures path: failures_zing-${{ matrix.java_version }}-${{ matrix.config }}-amd64.txt + - uses: actions/upload-artifact@v3 + if: failure() + with: + name: recordings + path: /tmp/*.jfr test-linux-glibc-zing-aarch64: strategy: @@ -787,7 +822,7 @@ jobs: run: | set +e export TEST_COMMIT=${{ github.sha }} - export TEST_CONFIGURATION=glibc/${{ matrix.java_version }} + export TEST_CONFIGURATION=glibc/zing/${{ matrix.java_version }}-aarch64 export LIBC=glibc export JAVA_TEST_HOME=$(pwd)/test_${{ matrix.java_version }}_jdk export JAVA_HOME=$JAVA_HOME @@ -803,7 +838,7 @@ jobs: printf "%s.%s.%s\n", v[1], v[2], v[3] } }') - ./gradlew :ddprof-test:test${{ matrix.config }} + ./gradlew -PkeepJFRs :ddprof-test:test${{ matrix.config }} --no-daemon --parallel --build-cache --no-watch-fs if [ $? -ne 0 ]; then echo "glibc-zing-${{ matrix.java_version }}-${{ matrix.config }}-aarch64" >> failures_zing-${{ matrix.java_version }}-${{ matrix.config }}-aarch64.txt exit 1 @@ -827,4 +862,9 @@ jobs: if: failure() with: name: failures - path: failures_zing-${{ matrix.java_version }}-${{ matrix.config }}-aarch64.txt \ No newline at end of file + path: failures_zing-${{ matrix.java_version }}-${{ matrix.config }}-aarch64.txt + - uses: actions/upload-artifact@v3 + if: failure() + with: + name: recordings + path: /tmp/*.jfr \ No newline at end of file diff --git a/README.md b/README.md index b2570252..982a3acc 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,16 @@ If you need a full-fledged Java profiler head back to [async-profiler](https://g Once prerequisites have been installed simple as `./gradlew assembleAll`. The resulting artifact is located in `ddprof-lib/build/libs/ddprof-.jar` +## Testing +The associated test cases can be run for each specific configuration: +- `testRelase` - test the release artifacts +- `testDebug` - test the debug artifacts +- `testAsan` - test with ASan +- `testTsan` - test with TSan + +The tests that create a JFR recording for the purpose of asserting will delete this recording at the end, unless `-PkeepJFRs` is provided to gradle. + +One can also completely skip all tests by adding `-Pskip-tests` gradle property. ### Consuming the artifact diff --git a/ddprof-lib/gtest/build.gradle b/ddprof-lib/gtest/build.gradle index f82a0b10..3c002fae 100644 --- a/ddprof-lib/gtest/build.gradle +++ b/ddprof-lib/gtest/build.gradle @@ -77,7 +77,7 @@ def buildSmallLibTask = tasks.register("buildSmallLib", Exec) { def gtestAll = tasks.register("gtest") { onlyIf { - hasGtest && !project.hasProperty('skip-native') + hasGtest && !project.hasProperty('skip-tests') && !project.hasProperty('skip-native') } group = 'verification' description = "Run all Google Tests for all build configurations of the library" @@ -98,7 +98,7 @@ tasks.whenTaskAdded { task -> def testName = it.name.substring(0, it.name.lastIndexOf('.')) def gtestCompileTask = tasks.register("compileGtest${config.name.capitalize()}_${testName}", CppCompile) { onlyIf { - config.active && hasGtest && !project.hasProperty('skip-native') + config.active && hasGtest && !project.hasProperty('skip-tests') && !project.hasProperty('skip-native') } group = 'build' description = "Compile the Google Test ${testName} for the ${config.name} build of the library" @@ -155,7 +155,7 @@ tasks.whenTaskAdded { task -> def binary = file("$buildDir/bin/gtest/${config.name}_${testName}/${testName}") def gtestLinkTask = tasks.register("linkGtest${config.name.capitalize()}_${testName}", LinkExecutable) { onlyIf { - config.active && hasGtest && !project.hasProperty('skip-native') + config.active && hasGtest && !project.hasProperty('skip-tests') && !project.hasProperty('skip-native') } group = 'build' description = "Link the Google Test for the ${config.name} build of the library" @@ -179,7 +179,7 @@ tasks.whenTaskAdded { task -> } def gtestExecuteTask = tasks.register("gtest${config.name.capitalize()}_${testName}", Exec) { onlyIf { - config.active && hasGtest && !project.hasProperty('skip-native') + config.active && hasGtest && !project.hasProperty('skip-tests') && !project.hasProperty('skip-native') } group = 'verification' description = "Run the Google Test ${testName} for the ${config.name} build of the library" diff --git a/ddprof-test/build.gradle b/ddprof-test/build.gradle index 9d5774d0..d0f4a131 100644 --- a/ddprof-test/build.gradle +++ b/ddprof-test/build.gradle @@ -78,7 +78,9 @@ tasks.withType(Test).configureEach { def config = it.name.replace("test", "") - jvmArgs '-Djdk.attach.allowAttachSelf', '-Djol.tryWithSudo=true', + def keepRecordings = hasProperty("keepJFRs") + + jvmArgs "-Dddprof_test.keep_jfrs=${keepRecordings}", '-Djdk.attach.allowAttachSelf', '-Djol.tryWithSudo=true', "-Dddprof_test.config=${config}", '-XX:ErrorFile=build/hs_err_pid%p.log', '-XX:+ResizeTLAB', '-Xmx512m' diff --git a/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java b/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java index 3a0219e3..010349cd 100644 --- a/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java +++ b/ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java @@ -1,5 +1,6 @@ package com.datadoghq.profiler; +import java.lang.reflect.Method; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.Paths; @@ -13,6 +14,7 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Assumptions; import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.TestInfo; import org.openjdk.jmc.common.IMCStackTrace; import org.openjdk.jmc.common.item.Attribute; @@ -145,11 +147,13 @@ protected boolean isPlatformSupported() { protected void withTestAssumptions() {} @BeforeEach - public void setupProfiler() throws Exception { + public void setupProfiler(TestInfo testInfo) throws Exception { Assumptions.assumeTrue(isPlatformSupported()); withTestAssumptions(); - jfrDump = Files.createTempFile(Paths.get("/tmp"), getClass().getName() + UUID.randomUUID(), ".jfr"); + String testConfig = System.getenv("TEST_CONFIGURATION"); + testConfig = testConfig == null ? "" : testConfig; + jfrDump = Files.createTempFile(Paths.get("/tmp"), testConfig.replace('/', '_') + "-" + testInfo.getTestMethod().map(Method::getName).orElse("unknown"), ".jfr"); profiler = JavaProfiler.getInstance(); String command = "start," + getAmendedProfilerCommand() + ",jfr,file=" + jfrDump.toAbsolutePath(); cpuInterval = command.contains("cpu") ? parseInterval(command, "cpu") : (command.contains("interval") ? parseInterval(command, "interval") : Duration.ZERO); @@ -164,7 +168,9 @@ public void setupProfiler() throws Exception { public void cleanup() throws Exception { after(); stopProfiler(); - // Files.deleteIfExists(jfrDump); + if (jfrDump != null && !Boolean.getBoolean("ddprof_test.keep_jfrs")) { + Files.deleteIfExists(jfrDump); + } } protected void before() throws Exception {