ci: count integration configurations with no result as failures #792
+231
−11
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,15 @@ RESULTS_DIR="${1:-integration-test-results}" | |
| # Dashboard URL (GitHub Pages) | ||
| DASHBOARD_URL="https://datadog.github.io/java-profiler/integration/" | ||
|
|
||
| # The matrix this run is expected to cover. Declared once: it drives both the | ||
| # completeness check and the table rendering below, so the two cannot drift. | ||
| PLATFORMS=( | ||
| glibc-x64-hotspot glibc-x64-openj9 glibc-arm64-hotspot glibc-arm64-openj9 | ||
| musl-x64-hotspot musl-x64-openj9 musl-arm64-hotspot musl-arm64-openj9 | ||
| ) | ||
| JDKS=(8 11 17 21 25) | ||
| EXPECTED=$(( ${#PLATFORMS[@]} * ${#JDKS[@]} )) | ||
|
|
||
| log_info "Collecting results for branch: ${DDPROF_COMMIT_BRANCH:-<unset>}" | ||
|
|
||
| # Collect test results | ||
|
|
@@ -95,8 +104,29 @@ done | |
|
|
||
| TOTAL=$((TOTAL_PASS + TOTAL_FAIL)) | ||
|
|
||
| # A configuration that produced no readable validation log is not a pass. Its | ||
| # job may have timed out, lost its runner, or failed in setup before writing | ||
| # one, which leaves its artifact directory empty -- or absent entirely, in | ||
| # which case the collection loop above never sees it. Walk the expected matrix | ||
| # so both shapes are caught, and gate on them: without this, 39 passes and one | ||
| # timed-out cell reports success. | ||
| TOTAL_INCOMPLETE=0 | ||
| INCOMPLETE_CONFIGS="" | ||
| for platform in "${PLATFORMS[@]}"; do | ||
| for jdk in "${JDKS[@]}"; do | ||
| config="${platform}-jdk${jdk}" | ||
| case "${RESULTS[${config}]:-missing}" in | ||
| pass|fail) ;; | ||
| *) | ||
| TOTAL_INCOMPLETE=$((TOTAL_INCOMPLETE + 1)) | ||
| INCOMPLETE_CONFIGS="${INCOMPLETE_CONFIGS} ${config}" | ||
| ;; | ||
| esac | ||
| done | ||
| done | ||
|
|
||
| # Determine overall status | ||
| if [ "${TOTAL_FAIL}" -gt 0 ] || [ "${TOTAL_INCOMPLETE}" -gt 0 ]; then | ||
| OVERALL_STATUS="failure" | ||
| STATUS_EMOJI=":x:" | ||
| STATUS_TEXT="FAILED" | ||
|
|
@@ -113,14 +143,17 @@ else | |
| STATUS_TEXT="COULD NOT RUN" | ||
| fi | ||
|
|
||
| log_info "Results: ${TOTAL_PASS} passed, ${TOTAL_FAIL} failed, ${TOTAL_INCOMPLETE} without a result, out of ${EXPECTED} expected configurations" | ||
| if [ -n "${INCOMPLETE_CONFIGS}" ]; then | ||
| log_warn "No validation log for:${INCOMPLETE_CONFIGS}" | ||
| fi | ||
|
|
||
| # Build the comment body | ||
| DDPROF_SHA="${DDPROF_COMMIT_SHA:-$(cat ddprof-commit-sha.txt 2>/dev/null || echo unknown)}" | ||
|
|
||
| if [ "${OVERALL_STATUS}" = "success" ]; then | ||
| # Every expected configuration passed - keep it short | ||
| COMMENT_BODY=":white_check_mark: **All ${TOTAL_PASS} integration tests passed** | ||
|
|
||
| :bar_chart: [Dashboard](${DASHBOARD_URL}) · :construction_worker: [Pipeline](${CI_PIPELINE_URL:-}) · :package: \`${DDPROF_SHA:0:8}\`" | ||
| elif [ "${TOTAL}" -eq 0 ]; then | ||
|
|
@@ -134,18 +167,28 @@ The test matrix in \`${RESULTS_DIR}\` is empty. This usually means a setup step | |
| :construction_worker: [Pipeline](${CI_PIPELINE_URL:-}) · :package: \`${DDPROF_SHA:0:8}\`" | ||
| else | ||
| # Some failures or unknowns - show full matrix | ||
| # Count against the expected matrix, not against the results that turned up: | ||
| # TOTAL is pass+fail, so a 40-cell matrix with one timed-out job would read | ||
| # "39 passed, 0 failed out of 39" while the pipeline failed and the section | ||
| # below explained the missing cell. The incomplete clause is omitted when | ||
| # there is nothing to report so the ordinary failure case stays terse. | ||
| HEADLINE="${STATUS_EMOJI} **${TOTAL_PASS}** passed, **${TOTAL_FAIL}** failed" | ||
| if [ "${TOTAL_INCOMPLETE}" -gt 0 ]; then | ||
| HEADLINE="${HEADLINE}, **${TOTAL_INCOMPLETE}** without a result" | ||
| fi | ||
| HEADLINE="${HEADLINE} out of **${EXPECTED}** expected configurations" | ||
|
|
||
| COMMENT_BODY="${HEADLINE} | ||
|
|
||
| ### Test Matrix | ||
|
|
||
| | Platform | JDK 8 | JDK 11 | JDK 17 | JDK 21 | JDK 25 | | ||
| |----------|-------|--------|--------|--------|--------|" | ||
|
|
||
| # Build matrix rows | ||
| for platform in "${PLATFORMS[@]}"; do | ||
| row="| ${platform} |" | ||
| for jdk in "${JDKS[@]}"; do | ||
| config="${platform}-jdk${jdk}" | ||
| status="${RESULTS[${config}]:-unknown}" | ||
| case "${status}" in | ||
|
|
@@ -158,6 +201,22 @@ else | |
| ${row}" | ||
| done | ||
|
|
||
| # Call out configurations that produced no result at all: a grey cell is not | ||
| # a pass, and the reason is not in any validation log. | ||
| if [ -n "${INCOMPLETE_CONFIGS}" ]; then | ||
| COMMENT_BODY="${COMMENT_BODY} | ||
|
|
||
| ### No result produced | ||
| These configurations wrote no validation log, so their outcome is unknown and | ||
| they are counted as failures. The job usually timed out or failed during setup; | ||
| see its log in the pipeline. | ||
|
Comment on lines
+209
to
+212
There was a problem hiding this comment.
When only part of the expected matrix is incomplete, these cells gate the pipeline but are not added to Useful? React with 👍 / 👎.
Contributor
Author
There was a problem hiding this comment. Valid — fixed in 3c86df5. The gating was right but the summary contradicted it: The headline now counts against the expected matrix and names the cells with no That matches the Covered by 14 assertions in Mutation-checked, 5 of 5 caught: reverting the denominator to |
||
| " | ||
| for config in ${INCOMPLETE_CONFIGS}; do | ||
| COMMENT_BODY="${COMMENT_BODY} | ||
| - \`${config}\`" | ||
| done | ||
| fi | ||
|
|
||
| # Add failure details if any | ||
| if [ -n "${FAILURES}" ]; then | ||
| COMMENT_BODY="${COMMENT_BODY} | ||
|
|
@@ -178,8 +237,14 @@ fi | |
| # Post comment via dd-octo-sts (upsert-github-pr-comment.sh handles missing | ||
| # branch/PR/token gracefully, so a comment-posting problem never masks the | ||
| # actual test outcome below). | ||
| # COMMENT_BODY_FILE lets a caller keep the rendered body instead of a temp file | ||
| # it never sees; the unit tests assert on it. | ||
| if [ -n "${COMMENT_BODY_FILE:-}" ]; then | ||
| BODY_FILE="${COMMENT_BODY_FILE}" | ||
| else | ||
| BODY_FILE=$(mktemp) | ||
| trap 'rm -f "${BODY_FILE}"' EXIT | ||
| fi | ||
| echo "${COMMENT_BODY}" > "${BODY_FILE}" | ||
| if ! "${HERE}/../scripts/upsert-github-pr-comment.sh" \ | ||
| "dd-trace-integration-results" "${DDPROF_COMMIT_BRANCH:-}" "${BODY_FILE}"; then | ||
|
|
@@ -188,7 +253,12 @@ fi | |
|
|
||
| # Exit with failure if tests failed (makes pipeline fail) | ||
| if [ "${OVERALL_STATUS}" = "failure" ]; then | ||
| if [ "${TOTAL_FAIL}" -gt 0 ]; then | ||
| log_error "${TOTAL_FAIL} integration test(s) failed - marking pipeline as failed" | ||
| fi | ||
| if [ "${TOTAL_INCOMPLETE}" -gt 0 ]; then | ||
| log_error "${TOTAL_INCOMPLETE} configuration(s) produced no result - marking pipeline as failed" | ||
| fi | ||
| exit 1 | ||
| fi | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,149 @@ | ||
| #! /bin/bash | ||
| # Minimal, dependency-free unit tests for the summary that | ||
| # .gitlab/dd-trace-integration/post-pr-comment.sh posts. | ||
| # Run with: bash .gitlab/dd-trace-integration/tests/post_pr_comment_test.sh | ||
| # | ||
| # The script reads a results directory and renders a comment; these tests build | ||
| # that directory, capture the rendered body through COMMENT_BODY_FILE, and | ||
| # assert on the headline counts and the exit status. Posting is left to fail on | ||
| # its own (no token in a test environment), which the script already tolerates. | ||
|
|
||
| set -eo pipefail | ||
|
|
||
| HERE=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd ) | ||
| SCRIPT="${HERE}/../post-pr-comment.sh" | ||
|
|
||
| # The matrix the script expects, mirrored here so a test can build a full one. | ||
| PLATFORMS=( | ||
| glibc-x64-hotspot glibc-x64-openj9 glibc-arm64-hotspot glibc-arm64-openj9 | ||
| musl-x64-hotspot musl-x64-openj9 musl-arm64-hotspot musl-arm64-openj9 | ||
| ) | ||
| JDKS=(8 11 17 21 25) | ||
|
|
||
| FAILED=0 | ||
| WORK=$(mktemp -d) | ||
| trap 'rm -rf "${WORK}"' EXIT | ||
|
|
||
| # Writes one config directory whose two validation logs say pass or fail. | ||
| write_config() { | ||
| local dir="$1" config="$2" outcome="$3" | ||
| local marker="SUCCESS:" | ||
| [ "${outcome}" = "fail" ] && marker="VALIDATION_FAILED" | ||
| mkdir -p "${dir}/${config}" | ||
| echo "${marker} whatever" > "${dir}/${config}/profiler-only-${config}.log" | ||
| echo "${marker} whatever" > "${dir}/${config}/tracer-profiler-${config}.log" | ||
| } | ||
|
|
||
| # Builds a full expected matrix, then applies per-config overrides: | ||
| # omit:<config> -> the directory is absent entirely (a lost/timed-out job) | ||
| # fail:<config> -> the logs report a validation failure | ||
| build_matrix() { | ||
| local dir="$1"; shift | ||
| local -a omit=() failing=() | ||
| local spec | ||
| for spec in "$@"; do | ||
| case "${spec}" in | ||
| omit:*) omit+=("${spec#omit:}") ;; | ||
| fail:*) failing+=("${spec#fail:}") ;; | ||
| esac | ||
| done | ||
| mkdir -p "${dir}" | ||
| local platform jdk config skip f | ||
| for platform in "${PLATFORMS[@]}"; do | ||
| for jdk in "${JDKS[@]}"; do | ||
| config="${platform}-jdk${jdk}" | ||
| skip=0 | ||
| for f in "${omit[@]:-}"; do [ "${f}" = "${config}" ] && skip=1; done | ||
| [ "${skip}" -eq 1 ] && continue | ||
| local outcome="pass" | ||
| for f in "${failing[@]:-}"; do [ "${f}" = "${config}" ] && outcome="fail"; done | ||
| write_config "${dir}" "${config}" "${outcome}" | ||
| done | ||
| done | ||
| } | ||
|
|
||
| # Runs the script over a results dir, keeping the rendered body and exit code. | ||
| run_script() { | ||
| local dir="$1" | ||
| BODY="${WORK}/body.md" | ||
| : > "${BODY}" | ||
| SCRIPT_EXIT=0 | ||
| COMMENT_BODY_FILE="${BODY}" "${BASH}" "${SCRIPT}" "${dir}" > "${WORK}/log.txt" 2>&1 || SCRIPT_EXIT=$? | ||
| } | ||
|
|
||
| assert_body_contains() { | ||
| local desc="$1" needle="$2" | ||
| if grep -qF -- "${needle}" "${BODY}"; then | ||
| echo "PASS: ${desc}" | ||
| else | ||
| echo "FAIL: ${desc} — body does not contain '${needle}'" | ||
| sed 's/^/ /' "${BODY}" | head -12 | ||
| FAILED=1 | ||
| fi | ||
| } | ||
|
|
||
| assert_body_lacks() { | ||
| local desc="$1" needle="$2" | ||
| if [ ! -s "${BODY}" ]; then | ||
| echo "FAIL: ${desc} — no comment body was rendered, so the absence proves nothing" | ||
| FAILED=1 | ||
| return | ||
| fi | ||
| if grep -qF -- "${needle}" "${BODY}"; then | ||
| echo "FAIL: ${desc} — body unexpectedly contains '${needle}'" | ||
| sed 's/^/ /' "${BODY}" | head -12 | ||
| FAILED=1 | ||
| else | ||
| echo "PASS: ${desc}" | ||
| fi | ||
| } | ||
|
|
||
| assert_exit() { | ||
| local desc="$1" expected="$2" | ||
| if [ "${SCRIPT_EXIT}" -eq "${expected}" ]; then | ||
| echo "PASS: ${desc}" | ||
| else | ||
| echo "FAIL: ${desc} — expected exit ${expected}, got ${SCRIPT_EXIT}" | ||
| sed 's/^/ /' "${WORK}/log.txt" | tail -8 | ||
| FAILED=1 | ||
| fi | ||
| } | ||
|
|
||
| # --- a complete, passing matrix --- | ||
|
|
||
| build_matrix "${WORK}/all-pass" | ||
| run_script "${WORK}/all-pass" | ||
| assert_exit "a complete passing matrix exits 0" 0 | ||
| assert_body_contains "it reports every configuration as passed" "All 40 integration tests passed" | ||
|
|
||
| # --- one cell produced no result at all: the case this gating exists for --- | ||
| # | ||
| # The headline has to account for it. Reporting "39 passed, 0 failed out of 39" | ||
| # while failing the pipeline tells a reviewer the opposite of what happened. | ||
|
|
||
| build_matrix "${WORK}/one-missing" "omit:musl-arm64-openj9-jdk25" | ||
| run_script "${WORK}/one-missing" | ||
| assert_exit "a missing cell fails the pipeline" 1 | ||
| assert_body_contains "the headline counts the passes" "**39** passed" | ||
| assert_body_contains "the headline counts zero failures" "**0** failed" | ||
| assert_body_contains "the headline counts the cell with no result" "**1** without a result" | ||
| assert_body_contains "the headline denominator is the expected matrix" "out of **40** expected configurations" | ||
| assert_body_contains "the missing cell is named" "musl-arm64-openj9-jdk25" | ||
|
|
||
| # --- an ordinary validation failure keeps the headline terse --- | ||
|
|
||
| build_matrix "${WORK}/one-fail" "fail:glibc-x64-hotspot-jdk11" | ||
| run_script "${WORK}/one-fail" | ||
| assert_exit "a validation failure fails the pipeline" 1 | ||
| assert_body_contains "the headline counts the failure" "**1** failed" | ||
| assert_body_contains "the denominator is still the expected matrix" "out of **40** expected configurations" | ||
| assert_body_lacks "no incomplete clause when nothing is missing" "without a result" | ||
|
|
||
| # --- nothing ran at all --- | ||
|
|
||
| mkdir -p "${WORK}/empty" | ||
| run_script "${WORK}/empty" | ||
| assert_exit "an empty results directory fails the pipeline" 1 | ||
| assert_body_contains "an empty matrix says the tests could not run" "no results were produced" | ||
|
|
||
| exit "${FAILED}" |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pipeline fails correctly, but the PR comment gives reviewers incorrect summary counts.
Assertion details
The comment must report 39 passed, 0 failed, and 1 without a result out of 40 expected configurations.The comment reports 39 passed, 0 failed out of 39 configurations. It lists the missing 40th configuration separately as a failure.Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest · Open Bits AI session
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Valid — fixed in 3c86df5.
The gating was right but the summary contradicted it:
TOTALispass+fail,computed before the completeness walk, so the headline only counted cells that
reported. For a change whose whole point is "a grey cell is not a pass", having
the summary say everything passed is worse than the original hole.
The headline now counts against the expected matrix and names the cells with no
result:
That matches the
log_infoline, which already reported all three numbersagainst
EXPECTED— so this makes the comment consistent with the log ratherthan introducing a new format. The incomplete clause is omitted when the count
is zero, so an ordinary validation failure reads exactly as before.
Covered by 14 assertions in
.gitlab/dd-trace-integration/tests/post_pr_comment_test.sh,run by the existing
shell-unit-testsjob: a complete matrix, one cell absententirely, one cell failing validation, and an empty results directory — each
asserting the rendered headline and the exit status.
post-pr-comment.shgained a
COMMENT_BODY_FILEhook so the body can be inspected instead of goingto a temp file the caller never sees.
Mutation-checked, 5 of 5 caught: reverting the denominator to
TOTAL, droppingthe incomplete clause, ungating the exit, not counting incomplete cells, and
not naming them each turn a named assertion red.