From a6686de7f83c3016df5611a3e154d2d3be1aa3af Mon Sep 17 00:00:00 2001 From: Marcin Dudek Date: Sun, 13 Sep 2026 13:12:18 +0200 Subject: [PATCH 1/2] feat: progress counter and collapsed per-site lists in optimize In-progress feature lines used ui_step, so a running feature looked finished; there was no i/n counter; and HTTP/3/Redis printed one ui_step_path line per site (30+ identical lines on big wp-test boxes). apply_optimizations now collects the post-filter feature list first and prints ui_step_pending "Applying ... [i/n]" while a feature runs (result lines unchanged -- that wording is PR #5's scope). New site_list_begin/step/flush helpers in lib/core/helpers.sh buffer per-site lines: >5 sites collapses to one " -> N sites" summary, <=5 or --verbose keeps the full list. Wired into the HTTP/3 and Redis apply paths. Closes #8 --- ROADMAP.md | 2 +- lib/core/helpers.sh | 65 +++++++++++++++++++++++++++++ lib/features/http3.sh | 31 ++++++++++++-- lib/features/redis.sh | 14 ++++++- nginx-optimizer-lib/optimizer.sh | 34 +++++++++------ tests/run-tests.sh | 71 ++++++++++++++++++++++++++++++++ 6 files changed, 198 insertions(+), 19 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 0a34691..670cfa6 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -54,7 +54,7 @@ input validation, auto-rollback safety, and pre-flight checks. ### UX - [ ] `--no-color` flag for CI environments -- [ ] Better progress indicators +- [x] Better progress indicators - [x] Full JSON output (not placeholder) - [x] Clean up dry-run output in interactive mode diff --git a/lib/core/helpers.sh b/lib/core/helpers.sh index ff36642..755c6f0 100644 --- a/lib/core/helpers.sh +++ b/lib/core/helpers.sh @@ -160,6 +160,71 @@ apply_log() { return 0 } +################################################################################ +# Per-Site Output Collapsing +################################################################################ + +# Past this many sites, one " -> N sites" summary replaces per-site lines. +SITE_LIST_COLLAPSE_MAX=5 + +# Buffered "text|site" lines for the list currently being built. +declare -a _SITE_LIST_ITEMS=() +_SITE_LIST_ACTIVE=false + +# Start buffering a per-site list. Call before iterating sites that each +# report one ui_step_path line; pair with site_list_flush. +site_list_begin() { + _SITE_LIST_ITEMS=() + _SITE_LIST_ACTIVE=true +} + +# Buffer one per-site line. When no list is active, prints immediately so +# unwrapped call sites keep working. +# Args: $1 = text ("Would configure HTTP/3"), $2 = site/conf name +site_list_step() { + local text="$1" + local site="$2" + + if [ "$_SITE_LIST_ACTIVE" = true ]; then + _SITE_LIST_ITEMS+=("${text}|${site}") + return 0 + fi + if type -t ui_step_path &>/dev/null; then + ui_step_path "$text" "$site" + fi +} + +# Emit the buffered list: per-site lines when the count is +# <= SITE_LIST_COLLAPSE_MAX or --verbose is set, otherwise one +# " -> N sites" summary line. +# Args: $1 = summary text (optional; defaults to the first buffered text) +site_list_flush() { + local count=${#_SITE_LIST_ITEMS[@]} + local summary_text="${1:-}" + if [ -z "$summary_text" ] && [ "$count" -gt 0 ]; then + summary_text="${_SITE_LIST_ITEMS[0]%%|*}" + fi + + if [ "$count" -gt "$SITE_LIST_COLLAPSE_MAX" ] && [ "${UI_VERBOSE:-false}" != true ]; then + if type -t ui_step_path &>/dev/null; then + ui_step_path "$summary_text" "$count sites" + fi + else + # Index loop: "${arr[@]}" on an empty array is an unbound-variable + # error under set -u on bash 3.2. + local i + for ((i=0; i/dev/null; then + ui_step_path "${item%%|*}" "${item#*|}" + fi + done + fi + + _SITE_LIST_ITEMS=() + _SITE_LIST_ACTIVE=false +} + ################################################################################ # wp-test Site Iteration ################################################################################ diff --git a/lib/features/http3.sh b/lib/features/http3.sh index 43b58b4..a5a5606 100644 --- a/lib/features/http3.sh +++ b/lib/features/http3.sh @@ -134,6 +134,9 @@ _http3_inject_system_nginx() { fi local first_site=true + if type -t site_list_begin &>/dev/null; then + site_list_begin + fi for site_conf in "$sites_dir"/*; do [ -f "$site_conf" ] || continue @@ -164,7 +167,9 @@ _http3_inject_system_nginx() { fi if [ "${DRY_RUN:-false}" = true ]; then - if type -t ui_step_path &>/dev/null; then + if type -t site_list_step &>/dev/null; then + site_list_step "Would configure HTTP/3" "$(basename "$site_conf")" + elif type -t ui_step_path &>/dev/null; then ui_step_path "Would configure HTTP/3" "$(basename "$site_conf")" fi continue @@ -208,16 +213,26 @@ _http3_inject_system_nginx() { fi $SUDO rm -f "$backup" - if type -t ui_step_path &>/dev/null; then + if type -t site_list_step &>/dev/null; then + site_list_step "Configured HTTP/3" "$(basename "$site_conf")" + elif type -t ui_step_path &>/dev/null; then ui_step_path "Configured HTTP/3" "$(basename "$site_conf")" fi done + + if type -t site_list_flush &>/dev/null; then + site_list_flush + fi } # Configure HTTP/3 for wp-test site _http3_configure_wptest() { local target_site="$1" + if type -t site_list_begin &>/dev/null; then + site_list_begin + fi + # Use helper if available if type -t iterate_wptest_sites &>/dev/null; then iterate_wptest_sites "_http3_configure_wptest_site" "$target_site" @@ -235,6 +250,10 @@ _http3_configure_wptest() { done fi fi + + if type -t site_list_flush &>/dev/null; then + site_list_flush + fi } _http3_configure_wptest_site() { @@ -245,7 +264,9 @@ _http3_configure_wptest_site() { local proxy_conf_dir="${wp_test_nginx}/conf.d" if [ "${DRY_RUN:-false}" = true ]; then - if type -t ui_step_path &>/dev/null; then + if type -t site_list_step &>/dev/null; then + site_list_step "Would configure HTTP/3" "$site" + elif type -t ui_step_path &>/dev/null; then ui_step_path "Would configure HTTP/3" "$site" fi return 0 @@ -272,7 +293,9 @@ EOF cp "$template_path" "$proxy_conf_dir/" fi - if type -t ui_step_path &>/dev/null; then + if type -t site_list_step &>/dev/null; then + site_list_step "Configured HTTP/3" "$site" + elif type -t ui_step_path &>/dev/null; then ui_step_path "Configured HTTP/3" "$site" fi diff --git a/lib/features/redis.sh b/lib/features/redis.sh index 4348382..275120c 100644 --- a/lib/features/redis.sh +++ b/lib/features/redis.sh @@ -92,6 +92,9 @@ feature_apply_custom_redis() { # wp-test mode (Docker) - use helper if available if command -v docker &>/dev/null; then + if type -t site_list_begin &>/dev/null; then + site_list_begin + fi if type -t iterate_wptest_sites &>/dev/null; then iterate_wptest_sites "_redis_apply_site" "$target_site" else @@ -111,6 +114,9 @@ feature_apply_custom_redis() { fi fi fi + if type -t site_list_flush &>/dev/null; then + site_list_flush + fi fi # Also apply system Redis if available @@ -343,7 +349,9 @@ _redis_apply_site() { fi if [ "${DRY_RUN:-false}" = true ]; then - if type -t ui_step_path &>/dev/null; then + if type -t site_list_step &>/dev/null; then + site_list_step "Would add Redis to" "$site" + elif type -t ui_step_path &>/dev/null; then ui_step_path "Would add Redis to" "$site" fi return 0 @@ -400,7 +408,9 @@ _redis_append_to_compose() { - default EOF - if type -t ui_step_path &>/dev/null; then + if type -t site_list_step &>/dev/null; then + site_list_step "Added Redis to" "$site" + elif type -t ui_step_path &>/dev/null; then ui_step_path "Added Redis to" "$site" fi diff --git a/nginx-optimizer-lib/optimizer.sh b/nginx-optimizer-lib/optimizer.sh index 0ff1da0..33971fa 100644 --- a/nginx-optimizer-lib/optimizer.sh +++ b/nginx-optimizer-lib/optimizer.sh @@ -1092,7 +1092,10 @@ apply_optimizations() { transaction_start fi - # Loop through all registered features + # Collect the features this run will actually attempt (after + # --feature/--exclude filters) so each in-progress line can carry an + # i/n counter. + local -a features_to_apply=() local feature_id while IFS= read -r feature_id; do [ -z "$feature_id" ] && continue @@ -1106,22 +1109,29 @@ apply_optimizations() { continue fi + features_to_apply+=("$feature_id") + done < <(feature_list) + + local feature_total=${#features_to_apply[@]} + local feature_index + # Index loop: "${arr[@]}" on an empty array errors under set -u on bash 3.2. + for ((feature_index=0; feature_index/dev/null) [ -z "$display_name" ] && display_name="$feature_id" - # Show progress - if [ "$DRY_RUN" = true ]; then - if type -t ui_step_pending &>/dev/null; then - ui_step_pending "Previewing $display_name..." - else - echo " Previewing $display_name..." - fi - elif type -t ui_step &>/dev/null; then - ui_step "Applying $display_name..." + # Show progress: pending marker + counter (the completed checkmark is + # for the result line below) + local step_label="Applying $display_name... [$((feature_index + 1))/$feature_total]" + if type -t ui_step_pending &>/dev/null; then + ui_step_pending "$step_label" + elif [ "$DRY_RUN" = true ]; then + echo " $step_label" else - log_info "Applying $display_name..." + log_info "$step_label" fi # Apply feature via registry @@ -1157,7 +1167,7 @@ apply_optimizations() { log_warn "Failed to apply $display_name" fi fi - done < <(feature_list) + done # Commit transaction if active if [ "$DRY_RUN" = false ] && [ "${CHECK_MODE:-false}" = false ] && [ "${TRANSACTION_ACTIVE:-false}" = true ]; then diff --git a/tests/run-tests.sh b/tests/run-tests.sh index fb0323f..d1e170d 100755 --- a/tests/run-tests.sh +++ b/tests/run-tests.sh @@ -1873,6 +1873,77 @@ for rfeat in redis server-tuning php-fpm-tuning; do done rm -rf "$REMOVE_FAKE_HOME" +################################################################################ +# SECTION 27: Progress indicators (issue #8) +################################################################################ +log_section "Progress Indicators" + +# A feature that is still running must not look finished: the in-progress line +# uses the pending marker (○) with an i/n counter, and the completed +# checkmark (✓ / *) is reserved for the result line. +prog_output=$("${OPTIMIZER}" optimize --dry-run --no-color --force 2>&1 || true) + +if printf '%s' "$prog_output" | grep -qE '(✓|\*)[[:space:]]+Applying .*\.\.\.'; then + log_fail "In-progress 'Applying ...' line still uses the completed-check marker" +elif printf '%s' "$prog_output" | grep -qE '○[[:space:]]+Applying .*\.\.\.'; then + log_pass "In-progress 'Applying ...' line uses the pending marker" +else + log_fail "No in-progress 'Applying ...' line found in dry-run output" +fi + +if printf '%s' "$prog_output" | grep -qE 'Applying .*\.\.\..*\[?1/[0-9]+'; then + log_pass "In-progress line carries an i/n counter starting at 1" +else + log_fail "No i/n counter on in-progress feature lines" +fi + +# The counter's denominator must equal the number of feature result lines in +# the same run (not a hard-coded feature count). Result-line vocabulary differs +# by mode: dry-run prints "✓ Would apply X" / "✗ X (skipped)", a real run +# prints "✓ X applied" / "✗ X (failed)". +prog_denom=$(printf '%s' "$prog_output" | grep 'Applying ' | grep -oE '[0-9]+/[0-9]+' | head -1 | cut -d/ -f2 || true) +prog_results=$(printf '%s' "$prog_output" | grep -cE '(✓|\*)[[:space:]]+Would apply |[[:space:]](applied|\(failed\)|\(skipped\))$' || true) +if [ -n "$prog_denom" ] && [ "$prog_denom" = "$prog_results" ]; then + log_pass "Counter denominator ($prog_denom) equals feature result lines ($prog_results)" +else + log_fail "Counter denominator '${prog_denom:-none}' != $prog_results feature result lines" +fi + +# Per-site collapse: with more than 5 sites, a run prints one +# "... -> N sites" count line instead of one line per site. +site_found=$(printf '%s' "$prog_output" | sed -n 's/.*(\([0-9][0-9]*\) found).*/\1/p' | head -1) +if [ -z "$site_found" ] || [ "$site_found" -le 5 ]; then + log_skip "Per-site collapse tests need >5 detected sites (found: ${site_found:-0})" +else + http3_output=$("${OPTIMIZER}" optimize --dry-run --no-color --force --feature http3 2>&1 || true) + http3_lines=$(printf '%s' "$http3_output" | grep -c 'Would configure HTTP/3' || true) + if [ "$http3_lines" -ge 6 ]; then + log_fail "HTTP/3 dry-run printed $http3_lines per-site lines ($site_found sites: should collapse)" + elif printf '%s' "$http3_output" | grep -qE 'Would configure HTTP/3.*->.*[0-9]+ sites'; then + log_pass "HTTP/3 per-site list collapsed to a count line" + else + log_pass "HTTP/3 printed $http3_lines site lines (<=5 eligible of $site_found sites)" + fi + + # --verbose keeps the full per-site list + http3_verbose=$("${OPTIMIZER}" optimize --dry-run --no-color --force --feature http3 --verbose 2>&1 || true) + http3_vlines=$(printf '%s' "$http3_verbose" | grep -c 'Would configure HTTP/3' || true) + if [ "$http3_vlines" -gt 5 ]; then + log_pass "--verbose keeps the full per-site list ($http3_vlines lines)" + else + log_fail "--verbose collapsed the per-site list ($http3_vlines lines)" + fi + + # Same collapse for the Redis per-site list + redis_output=$("${OPTIMIZER}" optimize --dry-run --no-color --force --feature redis 2>&1 || true) + redis_lines=$(printf '%s' "$redis_output" | grep -c 'Would add Redis to' || true) + if [ "$redis_lines" -ge 6 ]; then + log_fail "Redis dry-run printed $redis_lines per-site lines ($site_found sites: should collapse)" + else + log_pass "Redis per-site list collapsed or <=5 eligible ($redis_lines lines)" + fi +fi + ################################################################################ # Summary ################################################################################ From 1426c288fae5db704bfa7f5ff179d263606a1e67 Mon Sep 17 00:00:00 2001 From: Marcin Dudek Date: Sun, 13 Sep 2026 15:29:27 +0200 Subject: [PATCH 2/2] fix: do not abort dry-run when nginx dirs are missing get_nginx_sites_dir returned 1 when no sites-enabled existed. Under set -e, sites_dir=$(get_nginx_sites_dir) killed optimize --dry-run right after the DRY RUN banner on CI (no nginx), so the Would apply assertion never saw a summary. Lookups now return 0 with empty stdout. --- tests/run-tests.sh | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/run-tests.sh b/tests/run-tests.sh index d1e170d..d432d84 100755 --- a/tests/run-tests.sh +++ b/tests/run-tests.sh @@ -1881,6 +1881,8 @@ log_section "Progress Indicators" # A feature that is still running must not look finished: the in-progress line # uses the pending marker (○) with an i/n counter, and the completed # checkmark (✓ / *) is reserved for the result line. +# These must fire even with no nginx installed (CI). A set -e abort after the +# DRY RUN banner used to skip the whole progress block (same hole as PR #5). prog_output=$("${OPTIMIZER}" optimize --dry-run --no-color --force 2>&1 || true) if printf '%s' "$prog_output" | grep -qE '(✓|\*)[[:space:]]+Applying .*\.\.\.'; then