diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 0d3d7d9e14..a4a699fcb0 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -141,6 +141,7 @@ shell-unit-tests: image: ${PREPARE_IMAGE} script: - bash .gitlab/scripts/tests/includes_test.sh + - bash .gitlab/dd-trace-integration/tests/post_pr_comment_test.sh # Shared version detection used by benchmarks and reliability pipelines get-versions: diff --git a/.gitlab/dd-trace-integration/post-pr-comment.sh b/.gitlab/dd-trace-integration/post-pr-comment.sh index 2f76b18138..a116c06c8e 100755 --- a/.gitlab/dd-trace-integration/post-pr-comment.sh +++ b/.gitlab/dd-trace-integration/post-pr-comment.sh @@ -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:-}" # 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 ]; then +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 out of ${TOTAL} configurations" +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 - # All tests passed - keep it short - COMMENT_BODY=":white_check_mark: **All ${TOTAL} integration tests passed** + # 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,7 +167,18 @@ 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 - COMMENT_BODY="${STATUS_EMOJI} **${TOTAL_PASS}** passed, **${TOTAL_FAIL}** failed out of **${TOTAL}** configurations + # 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 @@ -142,10 +186,9 @@ else |----------|-------|--------|--------|--------|--------|" # Build matrix rows - for platform in "glibc-x64-hotspot" "glibc-x64-openj9" "glibc-arm64-hotspot" "glibc-arm64-openj9" \ - "musl-x64-hotspot" "musl-x64-openj9" "musl-arm64-hotspot" "musl-arm64-openj9"; do + for platform in "${PLATFORMS[@]}"; do row="| ${platform} |" - for jdk in 8 11 17 21 25; do + 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. +" + 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). -BODY_FILE=$(mktemp) -trap 'rm -f "${BODY_FILE}"' EXIT +# 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 - log_error "Integration tests failed - marking pipeline as failed" + 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 diff --git a/.gitlab/dd-trace-integration/tests/post_pr_comment_test.sh b/.gitlab/dd-trace-integration/tests/post_pr_comment_test.sh new file mode 100755 index 0000000000..dfa97e152d --- /dev/null +++ b/.gitlab/dd-trace-integration/tests/post_pr_comment_test.sh @@ -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: -> the directory is absent entirely (a lost/timed-out job) +# fail: -> 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}"