Skip to content

Commit 6462618

Browse files
kaahoskaahos
committed
fix: apply agent suggestions
1 parent 53fb055 commit 6462618

3 files changed

Lines changed: 69 additions & 44 deletions

File tree

‎README.md‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -170,16 +170,16 @@ By default, the script clones the repository at the current commit for clean bui
170170
./utils/run-containers-tests.sh --libc=musl --rebuild-base
171171

172172
# Preview all supported musl cells (sanitizer configs are skipped, matching CI policy)
173-
./utils/run-containers-tests.sh --libc=musl --jdk=all --arch=all
173+
./utils/run-containers-tests.sh --matrix --libc=musl
174174

175175
# Run all supported musl cells without an interactive prompt
176-
./utils/run-containers-tests.sh --libc=musl --jdk=all --arch=all --run
176+
./utils/run-containers-tests.sh --matrix --libc=musl --run
177177

178178
# Preview selected JDKs across every supported libc/architecture pair
179179
./utils/run-containers-tests.sh --libc=all --jdk=8,17,21 --arch=all
180180

181181
# Run all OpenJ9 cells
182-
./utils/run-containers-tests.sh --jdk=j9 --arch=all --run
182+
./utils/run-containers-tests.sh --matrix --jdk=j9 --run
183183

184184
# Show options
185185
./utils/run-containers-tests.sh --help
@@ -193,7 +193,7 @@ Supported options:
193193
- `--container=podman|docker` (default: podman)
194194
- `--tests="TestPattern"`
195195
- `--gtest` (enable C++ gtests, disabled by default for faster runs)
196-
- `--gtest-task=Task` (run one C++ gtest task; accepts `elfparser_ut` or a full task path like `:ddprof-lib:gtestAsan_elfparser_ut`)
196+
- `--gtest-task=Task` (run one C++ gtest task; matrix runs require a short name like `elfparser_ut`, while single-cell runs also accept a full task path like `:ddprof-lib:gtestAsan_elfparser_ut`)
197197
- `--shell` (interactive shell instead of running tests)
198198
- `--mount` (mount local repo instead of cloning - faster but may have stale artifacts)
199199
- `--rebuild` (force rebuild of all container images)
@@ -202,7 +202,7 @@ Supported options:
202202
- `--run` (execute an inferred matrix without prompting)
203203
- `--fail-fast` (stop matrix execution on first failure)
204204

205-
Single-value commands run one configuration immediately. When any dimension expands to multiple cells, the script prints a compact status table first; interactive terminals ask for confirmation, while non-interactive runs require `--run` to execute. Matrix execution prints the status table again after all cells finish and writes summaries to `build/reports/container-matrix/summary.md` and `build/reports/container-matrix/summary.json`.
205+
Single-value commands run one configuration immediately. When any dimension expands to multiple cells, the script prints a compact status table first; interactive terminals ask for confirmation, while non-interactive runs require `--run` to execute. Matrix execution prints the status table again after all cells finish and writes summaries to `build/reports/container-matrix/summary.md` and `build/reports/container-matrix/summary.json`. Cells not run because of `--fail-fast` are reported as cancelled separately from unsupported cells that are skipped.
206206

207207
## Unwinding Validation Tool
208208

‎utils/README.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -113,13 +113,13 @@ Examples:
113113
./utils/run-containers-tests.sh --libc=all --jdk=8,17,21 --arch=all
114114

115115
# Run all supported musl cells without an interactive prompt
116-
./utils/run-containers-tests.sh --libc=musl --jdk=all --arch=all --run
116+
./utils/run-containers-tests.sh --matrix --libc=musl --run
117117

118118
# Run all OpenJ9 cells
119-
./utils/run-containers-tests.sh --jdk=j9 --arch=all --run
119+
./utils/run-containers-tests.sh --matrix --jdk=j9 --run
120120
```
121121

122-
Single-value commands run one container test configuration immediately. When any dimension expands to multiple cells, the script prints a compact status table first; interactive terminals ask for confirmation, while non-interactive runs require `--run` to execute. Matrix execution prints the status table again after all cells finish and writes summaries to `build/reports/container-matrix/summary.md` and `build/reports/container-matrix/summary.json`.
122+
Single-value commands run one container test configuration immediately. When any dimension expands to multiple cells, the script prints a compact status table first; interactive terminals ask for confirmation, while non-interactive runs require `--run` to execute. Matrix execution prints the status table again after all cells finish and writes summaries to `build/reports/container-matrix/summary.md` and `build/reports/container-matrix/summary.json`. Matrix gtest runs require a short `--gtest-task` name so the task follows each cell's configuration. Cells not run because of `--fail-fast` are reported as cancelled separately from unsupported cells that are skipped.
123123

124124
### `patch-dd-java-agent.sh`
125125

‎utils/run-containers-tests.sh‎

Lines changed: 61 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@
1616
# --container=podman|docker (default: podman)
1717
# --tests="TestPattern" (optional, specific test to run)
1818
# --gtest (enable C++ gtests, disabled by default)
19-
# --gtest-task=Task (run one C++ gtest task; accepts elfparser_ut or :ddprof-lib:gtestAsan_elfparser_ut)
19+
# --gtest-task=Task (run one C++ gtest task; full task paths are single-cell only)
2020
# --shell (drop to shell instead of running tests; enables SYS_PTRACE for gdb)
2121
# --mount (mount local repo instead of cloning - faster but may have stale artifacts)
2222
# --rebuild (force rebuild of container images)
@@ -204,6 +204,19 @@ contains_value() {
204204
return 1
205205
}
206206

207+
validate_dimension_value() {
208+
local name=$1
209+
local value=$2
210+
211+
case "$value" in
212+
""|,*|*,|*,,*)
213+
echo "Error: --$name contains an empty value" >&2
214+
exit 1
215+
;;
216+
esac
217+
}
218+
219+
# Bash 3.2 lacks namerefs, so these helpers use eval with array names supplied only by internal callers; values remain quoted.
207220
append_unique_line() {
208221
local array_name=$1
209222
local value=$2
@@ -212,15 +225,6 @@ append_unique_line() {
212225
eval "$array_name+=(\"\$value\")"
213226
}
214227

215-
read_lines_into_array() {
216-
local array_name=$1
217-
local line
218-
eval "$array_name=()"
219-
while IFS= read -r line; do
220-
eval "$array_name+=(\"\$line\")"
221-
done
222-
}
223-
224228
read_string_into_array() {
225229
local array_name=$1
226230
local values=$2
@@ -238,7 +242,9 @@ expand_dimension() {
238242
local all_values=("$@")
239243
local expanded=()
240244
local token
245+
local tokens=()
241246

247+
validate_dimension_value "$name" "$value"
242248
IFS=',' read -ra tokens <<< "$value"
243249
for token in "${tokens[@]}"; do
244250
if [[ -z "$token" ]]; then
@@ -268,7 +274,9 @@ expand_jdk_dimension() {
268274
local all_jdks=("${regular_jdks[@]}" "${j9_jdks[@]}" "${graal_jdks[@]}")
269275
local expanded=()
270276
local token
277+
local tokens=()
271278

279+
validate_dimension_value "jdk" "$value"
272280
IFS=',' read -ra tokens <<< "$value"
273281
for token in "${tokens[@]}"; do
274282
case "$token" in
@@ -318,30 +326,21 @@ skip_reason() {
318326
local libc=$1
319327
local jdk=$2
320328
local config=$3
329+
local arch=$4
321330

322331
if [[ "$libc" == "musl" && "$jdk" == *-j9 ]]; then
323332
echo "J9/OpenJ9 is not available for musl libc"
324333
elif [[ "$libc" == "musl" && "$jdk" == *-graal ]]; then
325334
echo "GraalVM is not available for musl libc"
335+
elif [[ "$jdk" == *-graal && "$config" == "asan" ]]; then
336+
echo "GraalVM is not compatible with ASan shadow memory ranges"
337+
elif [[ "$libc" == "glibc" && "$arch" == "aarch64" && ( "$jdk" == "8" || "$jdk" == "11" ) ]]; then
338+
echo "HotSpot 8/11 are skipped on glibc aarch64 in CI"
326339
elif [[ "$libc" == "musl" && ( "$config" == "asan" || "$config" == "tsan" ) ]]; then
327340
echo "sanitizer configs are filtered out for musl in CI"
328341
fi
329342
}
330343

331-
matrix_command() {
332-
local libc=$1
333-
local arch=$2
334-
local jdk=$3
335-
local config=$4
336-
local cmd=("./utils/run-containers-tests.sh" "--libc=$libc" "--arch=$arch" "--jdk=$jdk" "--config=$config")
337-
338-
if [[ ${#PASS_THROUGH_ARGS[@]} -gt 0 ]]; then
339-
cmd+=("${PASS_THROUGH_ARGS[@]}")
340-
fi
341-
342-
printf "%q " "${cmd[@]}"
343-
}
344-
345344
cell_status_symbol() {
346345
local status=$1
347346
local style=${2:-text}
@@ -448,7 +447,7 @@ print_table_separator() {
448447

449448
print_matrix_status_table() {
450449
local style=${1:-text}
451-
local i row col cell_value
450+
local i j row col cell_value
452451
local columns=()
453452
local rows=()
454453
local jdk_width=3
@@ -525,6 +524,21 @@ print_failed_cells() {
525524
done
526525
}
527526

527+
print_skipped_cells() {
528+
local i has_skips=false
529+
530+
for ((i = 0; i < ${#MATRIX_CELL_LIBCS[@]}; i++)); do
531+
if [[ "${MATRIX_CELL_STATUSES[$i]}" == "skipped" ]]; then
532+
if ! $has_skips; then
533+
echo
534+
echo "Skipped cells:"
535+
has_skips=true
536+
fi
537+
echo "- jdk=${MATRIX_CELL_JDKS[$i]}, libc=${MATRIX_CELL_LIBCS[$i]}, arch=${MATRIX_CELL_ARCHES[$i]}, config=${MATRIX_CELL_CONFIGS[$i]}: ${MATRIX_CELL_REASONS[$i]}"
538+
fi
539+
done
540+
}
541+
528542
prepare_matrix_cells() {
529543
local libc arch jdk config reason
530544

@@ -544,7 +558,7 @@ prepare_matrix_cells() {
544558
for jdk in "${MATRIX_JDKS[@]}"; do
545559
for config in "${MATRIX_CONFIGS[@]}"; do
546560
((MATRIX_TOTAL += 1))
547-
reason=$(skip_reason "$libc" "$jdk" "$config")
561+
reason=$(skip_reason "$libc" "$jdk" "$config" "$arch")
548562
if [[ -n "$reason" ]]; then
549563
((MATRIX_SKIPPED += 1))
550564
MATRIX_CELL_STATUSES+=("skipped")
@@ -580,6 +594,7 @@ print_matrix_preview() {
580594
echo "Skipped cells: $MATRIX_SKIPPED"
581595
echo
582596
print_matrix_status_table emoji
597+
print_skipped_cells
583598
}
584599

585600
write_matrix_reports() {
@@ -588,6 +603,7 @@ write_matrix_reports() {
588603
local passed=$3
589604
local failed=$4
590605
local skipped=$5
606+
local cancelled=$6
591607
local report_dir="$PROJECT_ROOT/build/reports/container-matrix"
592608
local markdown_report="$report_dir/summary.md"
593609
local json_report="$report_dir/summary.json"
@@ -602,9 +618,12 @@ write_matrix_reports() {
602618
echo "- Started: $start_time"
603619
echo "- Finished: $end_time"
604620
echo "- Filters: libc=$LIBC, arch=$ARCH, jdk=$JDK_VERSION, config=$CONFIG"
605-
echo "- Totals: total=$MATRIX_TOTAL, passed=$passed, failed=$failed, skipped=$skipped"
621+
echo "- Totals: total=$MATRIX_TOTAL, passed=$passed, failed=$failed, skipped=$skipped, cancelled=$cancelled"
606622
echo
623+
echo '```text'
607624
print_matrix_status_table emoji
625+
echo '```'
626+
echo
608627
print_matrix_legend
609628
print_failed_cells
610629
echo
@@ -621,8 +640,8 @@ write_matrix_reports() {
621640
printf ' "finished": "%s",\n' "$(json_escape "$end_time")"
622641
printf ' "filters": {"libc": "%s", "arch": "%s", "jdk": "%s", "config": "%s"},\n' \
623642
"$(json_escape "$LIBC")" "$(json_escape "$ARCH")" "$(json_escape "$JDK_VERSION")" "$(json_escape "$CONFIG")"
624-
printf ' "totals": {"total": %d, "passed": %d, "failed": %d, "skipped": %d},\n' \
625-
"$MATRIX_TOTAL" "$passed" "$failed" "$skipped"
643+
printf ' "totals": {"total": %d, "passed": %d, "failed": %d, "skipped": %d, "cancelled": %d},\n' \
644+
"$MATRIX_TOTAL" "$passed" "$failed" "$skipped" "$cancelled"
626645
echo ' "cells": ['
627646
for ((i = 0; i < ${#MATRIX_CELL_LIBCS[@]}; i++)); do
628647
printf ' {"libc": "%s", "arch": "%s", "jdk": "%s", "config": "%s", "status": "%s", "exit_code": %s, "reason": "%s"}' \
@@ -676,7 +695,7 @@ confirm_matrix_run() {
676695

677696
run_matrix() {
678697
local i j command exit_code
679-
local passed=0 failed=0 skipped=$MATRIX_SKIPPED overall_exit=0
698+
local passed=0 failed=0 skipped=$MATRIX_SKIPPED cancelled=0 overall_exit=0
680699
local start_time end_time
681700

682701
if ! command -v "$CONTAINER_RUNTIME" >/dev/null 2>&1; then
@@ -717,9 +736,9 @@ run_matrix() {
717736
echo ">>> Stopping after first failure because --fail-fast is set"
718737
for ((j = i + 1; j < ${#MATRIX_CELL_LIBCS[@]}; j++)); do
719738
if [[ "${MATRIX_CELL_STATUSES[$j]}" == "pending" ]]; then
720-
MATRIX_CELL_STATUSES[$j]="skipped"
739+
MATRIX_CELL_STATUSES[$j]="cancelled"
721740
MATRIX_CELL_REASONS[$j]="not run because --fail-fast stopped after an earlier failure"
722-
((skipped += 1))
741+
((cancelled += 1))
723742
fi
724743
done
725744
break
@@ -729,13 +748,14 @@ run_matrix() {
729748
set -e
730749

731750
end_time=$(date -u +"%Y-%m-%dT%H:%M:%SZ")
732-
write_matrix_reports "$start_time" "$end_time" "$passed" "$failed" "$skipped"
751+
write_matrix_reports "$start_time" "$end_time" "$passed" "$failed" "$skipped" "$cancelled"
733752

734753
echo
735754
echo "=== Matrix Result ==="
736755
echo "Passed: $passed"
737756
echo "Failed: $failed"
738757
echo "Skipped: $skipped"
758+
echo "Cancelled: $cancelled"
739759
echo "====================="
740760
echo
741761
print_matrix_status_table emoji
@@ -851,8 +871,8 @@ if $MATRIX_MODE; then
851871
fi
852872
fi
853873

854-
# Auto-detect architecture if not specified
855-
if [[ -z "$ARCH" ]]; then
874+
# Auto-detect architecture only when neither --arch nor --matrix selected it.
875+
if ! $ARCH_SET && ! $MATRIX_MODE; then
856876
ARCH=$(detect_arch)
857877
fi
858878

@@ -876,14 +896,19 @@ read_string_into_array MATRIX_JDKS "$MATRIX_JDKS_TEXT"
876896
read_string_into_array MATRIX_CONFIGS "$MATRIX_CONFIGS_TEXT"
877897
prepare_matrix_cells
878898

899+
if [[ "$GTEST_TASK" == :* ]] && { (( MATRIX_TOTAL > 1 || MATRIX_RUNNABLE > 1 )) || $MATRIX_MODE; }; then
900+
echo "Error: full --gtest-task paths are not supported with matrix runs. Use a short task name such as --gtest-task=elfparser_ut and select configurations with --config."
901+
exit 1
902+
fi
903+
879904
if (( MATRIX_RUNNABLE == 0 )); then
880905
print_matrix_preview
881906
echo
882907
echo "Error: matrix has no runnable cells"
883908
exit 1
884909
fi
885910

886-
if (( MATRIX_TOTAL > 1 || MATRIX_RUNNABLE > 1 || MATRIX_MODE )); then
911+
if (( MATRIX_TOTAL > 1 || MATRIX_RUNNABLE > 1 )) || $MATRIX_MODE; then
887912
if $SHELL_MODE; then
888913
echo "Error: --shell is interactive and is not supported with matrix runs. Run a single cell without matrix dimensions instead."
889914
exit 1

0 commit comments

Comments
 (0)