From f7396b282a2567c94100f948a5f61da666c28d10 Mon Sep 17 00:00:00 2001 From: Miguel de Benito Delgado Date: Mon, 9 Nov 2026 12:14:40 +0000 Subject: [PATCH] :bug: Explain saved renderer A/B failures Print compact A/B failure summaries from the last saved comparison when the command returns status 1 with an empty error list. Include reasons and run, build or comparison references, with bounded fields and an eight-entry limit. Read interruption state after execution and cover saved configuration failures, unacknowledged differences and signal output in tests. AI-assisted-by: gpt-6.1-sol --- .../performance/benches/render_wasm/README.md | 10 +++- .../render_wasm/node/cli/deferred.cljs | 15 +++-- .../benches/render_wasm/report/format.cljs | 53 +++++++++++++++++ .../render_wasm/cli_integration.cjs | 12 +++- .../render_wasm/feature_comparison_test.cljs | 57 ++++++++++++++++++- .../render_wasm/run_test.cljs | 47 ++++++++++++++- 6 files changed, 184 insertions(+), 10 deletions(-) diff --git a/frontend/performance/benches/render_wasm/README.md b/frontend/performance/benches/render_wasm/README.md index aa9f373d8c..9a18f2e7ba 100644 --- a/frontend/performance/benches/render_wasm/README.md +++ b/frontend/performance/benches/render_wasm/README.md @@ -1191,8 +1191,11 @@ warnings. The runner validates the record with a schema and writes it atomically. It writes the initial plan before the first build and updates the record after each configuration, after each edge, and at termination. The command prints `Comparison `, `Output `, and `Status ` -on stdout at termination. Progress and failures go to stderr. Each configuration -has its own build and run log. +on stdout at termination. When a failed outcome has an empty error list, the +command appends a `Failures` section with saved reasons and artefact references. +The section shows up to eight failed configurations and comparisons, then names +the saved record and log with the complete evidence. Progress and runtime +failures go to stderr. Each configuration has its own build and run log. ### Edge comparisons @@ -1361,7 +1364,8 @@ The commands write results to stdout: - `run`: `Run ` and `Output ` after the run ends. When a report is available, a `Report` heading and the formatted summary follow. - `ab`: `Comparison `, `Output `, and final - `Status `. + `Status `. A failed outcome with an empty error list also prints a + bounded `Failures` section from the saved comparison record. - `compare`: the comparison report. - `merge`: `Run ` and `Output `. - `--help` and `COMMAND --help`: usage text with that command's options and diff --git a/frontend/performance/benches/render_wasm/node/cli/deferred.cljs b/frontend/performance/benches/render_wasm/node/cli/deferred.cljs index 122fa2d64c..3cda492436 100644 --- a/frontend/performance/benches/render_wasm/node/cli/deferred.cljs +++ b/frontend/performance/benches/render_wasm/node/cli/deferred.cljs @@ -14,7 +14,8 @@ [benches.render-wasm.node.runner.build :as build] [benches.render-wasm.node.runner.feature-comparison :as feature-comparison] [benches.render-wasm.node.runner.run :as run] - [benches.render-wasm.node.storage.artifacts :as artifacts])) + [benches.render-wasm.node.storage.artifacts :as artifacts] + [benches.render-wasm.report.format :as format])) (defn- ^:async build-command [{:keys [get-root signal]} options] @@ -45,14 +46,20 @@ (if (:dry-run options) (outcome/succeeded {:report (feature-comparison/format-plan prepared)}) (let [{:keys [home exit-code] :as result} - (await (feature-comparison/execute root prepared signal {:services services}))] + (await (feature-comparison/execute root prepared signal {:services services})) + interrupted (.-aborted signal) + failures (when (and home (= 1 exit-code) (not interrupted) + (= :failed (get-in result [:comparison :status])) + (empty? (:errors result))) + (format/format-feature-comparison-failures (:comparison result) home))] (cond-> result home (assoc :report (str "Comparison " (:comparison-id home) "\nOutput " (:dir home) "\nStatus " (cond - (.-aborted signal) "interrupted" + interrupted "interrupted" (zero? exit-code) "completed" - :else "failed")))))))) + :else "failed") + (when failures (str "\n" failures))))))))) (def ^:private handlers {"build" build-command "run" run-command "ab" ab-command}) diff --git a/frontend/performance/benches/render_wasm/report/format.cljs b/frontend/performance/benches/render_wasm/report/format.cljs index dcb82bd870..0150e2f0e8 100644 --- a/frontend/performance/benches/render_wasm/report/format.cljs +++ b/frontend/performance/benches/render_wasm/report/format.cljs @@ -10,6 +10,7 @@ Text leads with observed median, bootstrap interval and distinct counts. Full statistics and acknowledged differences include units and phase identity." (:require + [benches.render-wasm.errors :as errors] [benches.render-wasm.result :as result] [clojure.string :as str])) @@ -95,6 +96,58 @@ "\n preparations " (pr-str preparations) (apply str (map (fn [[id metric]] (str "\n" (metric->text id metric))) metrics)))) +(defn- quoted-detail + [value] + (errors/bounded-text (js/JSON.stringify value) 1024)) + +(defn- failure->text + [{:keys [failure reason]}] + (if failure + (if-let [n (:unacknowledged-differences failure)] + (str n " unacknowledged " (if (= n 1) "difference" "differences")) + (str (quoted-detail (:phase failure)) ": " (quoted-detail (:message failure)))) + (quoted-detail reason))) + +(defn- configuration-failure->text + [{:keys [slug run build] :as configuration}] + (str " Configuration " (quoted-detail slug) ": " (failure->text configuration) + (cond + run (str "\n Run record " (quoted-detail (:record-path run)) + "\n Log " (quoted-detail (:log-path run))) + build (str "\n Build manifest " (quoted-detail (:manifest-path build)) + "\n Log " (quoted-detail (:log-path build)))))) + +(defn- edge-failure->text + [{:keys [from to feature path] :as edge}] + (str " Comparison " (quoted-detail from) " -> " (quoted-detail to) + " (feature " (quoted-detail feature) "): " (failure->text edge) + (when path (str "\n Report " (quoted-detail path))))) + +(defn format-feature-comparison-failures + "Formats failed configurations and comparisons with artifact references. + + Args: + record: Validated feature-comparison record. + home: Comparison :record-path and :log-path. + + Returns: + Text for up to eight failures in configuration and comparison order, + with an omitted count and references to the complete evidence. String + fields escape C0 controls (U+0000 through U+001F) and use at most + 1024 UTF-16 units. + Returns nil when the record contains no failed entries." + [record home] + (let [configurations (filter #(= :failed (:status %)) (:configurations record)) + edges (filter #(= :failed (:status %)) (:edges record)) + total (+ (count configurations) (count edges)) + entries (concat (map configuration-failure->text configurations) + (map edge-failure->text edges))] + (when (pos? total) + (str "Failures\n" (str/join "\n" (take 8 entries)) + (when (> total 8) (str "\n " (- total 8) " more failures in the saved record")) + "\n Record " (quoted-detail (:record-path home)) + "\n Log " (quoted-detail (:log-path home)))))) + (defn format-comparison "Formats selected permissions, acknowledged/unacknowledged differences and changes. Both saved definitions remain visible. Edge labels describe actual feature diff --git a/frontend/test/performance_tests/render_wasm/cli_integration.cjs b/frontend/test/performance_tests/render_wasm/cli_integration.cjs index 8b42d79531..117e5b9548 100644 --- a/frontend/test/performance_tests/render_wasm/cli_integration.cjs +++ b/frontend/test/performance_tests/render_wasm/cli_integration.cjs @@ -921,6 +921,7 @@ test( ]); assert.equal(result.exitCode, 1); assert.equal(result.stdout.split("Status failed\n").length - 1, 1); + assert.equal(result.stdout.trim().split("\n").length, 3); assert.match(result.stderr, /✖ renderer build preparation failed/); }, ); @@ -961,8 +962,12 @@ test( if (child.exitCode === null) child.kill("SIGKILL"); }); const exited = once(child, "exit"); + const closed = once(child, "close"); + let stdout = ""; let stderr = ""; - child.stdout.resume(); + child.stdout.on("data", (chunk) => { + stdout += chunk; + }); child.stderr.on("data", (chunk) => { stderr += chunk; }); @@ -976,6 +981,11 @@ test( assert.ok(fs.existsSync(marker), stderr); child.kill(signal); assert.deepEqual(await exited, [code, null], stderr); + await closed; + if (command === "ab") { + assert.match(stdout, /^Status interrupted$/m); + assert.equal(stdout.trim().split("\n").length, 3); + } if (failPublication) assert.match(stderr, /publication unavailable/); } } diff --git a/frontend/test/performance_tests/render_wasm/feature_comparison_test.cljs b/frontend/test/performance_tests/render_wasm/feature_comparison_test.cljs index b42a6559fb..c84ca0f8c0 100644 --- a/frontend/test/performance_tests/render_wasm/feature_comparison_test.cljs +++ b/frontend/test/performance_tests/render_wasm/feature_comparison_test.cljs @@ -22,6 +22,7 @@ [benches.render-wasm.node.process :as process] [benches.render-wasm.node.runner.build :as build] [benches.render-wasm.node.runner.feature-comparison :as feature-comparison] + [benches.render-wasm.report.format :as format] [benches.render-wasm.result :as result] [cljs.test :as t :include-macros true] [clojure.string :as str] @@ -982,17 +983,71 @@ {:digest "a"})))})})) record (:comparison result) edge (first (:edges record)) - saved (result/decode (fs/readFileSync (:path edge) "utf8"))] + saved (result/decode (fs/readFileSync (:path edge) "utf8")) + text (format/format-feature-comparison-failures record (:home result))] (t/is (= 1 (:exit-code result))) + (t/is (empty? (:errors result))) (t/is (= :failed (:status record))) (t/is (= :failed (:status edge))) (t/is (fs/existsSync (:path edge))) (t/is (= {:unacknowledged-differences 1} (:failure edge))) (t/is (= :comparison (:type saved))) (t/is (= ["a"] (get-in saved [:edge :added]))) + (t/is (str/includes? text "Comparison \"0\" -> \"1\" (feature \"a\"): 1 unacknowledged difference")) + (t/is (str/includes? text (:path edge))) + (t/is (str/includes? text (get-in result [:home :record-path]))) (t/is (some #(and (= :fingerprint (:field %)) (not (:waived? %))) (get-in saved [:pairs 0 :mismatches]))))) +(t/deftest failure-summary-reports-build-reasons-and-comparison-evidence + (let [record (-> (terminal-record :failed) + (update-in [:configurations 0] dissoc :run :reason) + (assoc-in [:configurations 0 :failure] {:phase "catalog" :message "catalog rejected"}) + (assoc-in [:configurations 1 :status] :failed) + (assoc-in [:configurations 1 :reason] "run did not complete") + result/check-record) + home {:record-path "/feature-comparison.transit" :log-path "/feature-comparison.log"} + text (format/format-feature-comparison-failures record home) + edge (first (:edges record))] + (t/is (str/includes? text "Configuration \"00\": \"catalog\": \"catalog rejected\"")) + (t/is (str/includes? text "Configuration \"01\": \"run did not complete\"")) + (t/is (str/includes? text (str "Comparison " (pr-str (:from edge)) " -> " (pr-str (:to edge))))) + (t/is (str/includes? text "\"comparison\": \"partial\"")) + (t/is (str/includes? text "/build.transit")) + (t/is (str/includes? text "/build.log")) + (t/is (str/includes? text "/comparison.transit")) + (t/is (str/includes? text (:record-path home))) + (t/is (str/includes? text (:log-path home))))) + +(t/deftest failure-summary-bounds-and-escapes-details-and-names-omitted-evidence + (let [plan (feature-plan/plan ["a" "b" "c"] {:dry-run true}) + message (str "detail\n\u001b" (apply str (repeat 10000 "x")) "😀") + record (-> (planned-record) + (assoc :status :failed :ended-at "end" + :features (:features plan) + :configurations (mapv (fn [[index configuration]] + (if (< index 4) + (assoc configuration :status :failed + :failure {:phase "run" :message message}) + (completed-configuration configuration))) + (map-indexed vector (:configurations plan))) + :edges (mapv #(assoc % :status :failed :reason message) (:edges plan))) + (assoc-in [:options :features] (:features plan)) + result/check-record) + home {:record-path "/saved\n\u001b.transit" :log-path "/saved.log"} + text (format/format-feature-comparison-failures record home) + lines (str/split-lines text) + detail (second (re-find #"(?m)^ Configuration \"000\": \"run\": (.+)$" text))] + (t/is (= 8 (count (re-seq #"(?m)^ (?:Configuration|Comparison) " text)))) + (t/is (= 4 (count (re-seq #"(?m)^ Comparison " text)))) + (t/is (str/includes? text "8 more failures in the saved record")) + (t/is (str/includes? text "Record \"/saved")) + (t/is (string? detail)) + (t/is (<= (count detail) 1024)) + (t/is (every? #(<= (count %) 1100) lines)) + (t/is (every? #(>= (.charCodeAt % 0) 32) (mapcat seq lines))) + (t/is (< (count text) 12000)))) + (t/deftest ^:async interruption-skips-comparisons-and-returns-the-signal-exit (create-frontend-root) (let [prepared (comparison-prepared ["a"] {}) diff --git a/frontend/test/performance_tests/render_wasm/run_test.cljs b/frontend/test/performance_tests/render_wasm/run_test.cljs index ecd5999951..c9fa0189b6 100644 --- a/frontend/test/performance_tests/render_wasm/run_test.cljs +++ b/frontend/test/performance_tests/render_wasm/run_test.cljs @@ -12,6 +12,7 @@ [app.common.transit :as transit] [benches.render-wasm.errors :as errors] [benches.render-wasm.features.plan :as feature-plan] + [benches.render-wasm.node.cli.deferred :as deferred] [benches.render-wasm.node.cli.request :as request] [benches.render-wasm.node.log :as log] [benches.render-wasm.node.runner.browser :as browser] @@ -21,7 +22,8 @@ [benches.render-wasm.node.storage.runs :as runs] [benches.render-wasm.result :as result] [cljs.test :as t :include-macros true] - [clojure.string :as str])) + [clojure.string :as str] + [frontend-tests.helpers.mock :as mock])) (def ^:private temporary-root (atom nil)) @@ -1269,6 +1271,49 @@ (t/is (= :completed (get-in run [:termination :reason]))) (t/is (= 1 (count (:attempts run)))))))) +(t/deftest ^:async failed-feature-configuration-prints-its-saved-cause-and-log + (let [root @temporary-root + cases [(collected :case/a :fresh)] + fake (fake-browser {:close-page-failure #(= % 1)}) + prepared {:plan (feature-plan/plan ["a"] {:dry-run true}) + :cases cases + :output-dir nil + :options (assoc (dissoc (options nil nil) :browser) :filter "case/")} + response (await + (feature-comparison/execute + root prepared nil + {:services + {:prepare-build prepare-comparison-build + :collect-catalog (fn [_ _] cases) + :execute-run + (fn [root home selected cases opts signal] + (run/execute root home selected cases + (assoc opts :browser (:service fake)) signal))}})) + home (:home response) + saved (result/decode (fs/readFileSync (:record-path home) "utf8")) + config (first (:configurations saved)) + request (request/apply-defaults {:command "ab" :console {:color :never} + :options {:features ["a"] :filter "case/"}})] + (t/is (= 1 (:exit-code response))) + (t/is (empty? (:errors response))) + (t/is (= [:failed :completed] (mapv :status (:configurations saved)))) + (t/is (= :browser-close (get-in config [:failure :stage]))) + (t/is (every? #(= :completed (:status %)) (:edges saved))) + (await + (mock/with-mocks* + {feature-comparison/prepare (fn [_ _ _] (js/Promise.resolve prepared)) + feature-comparison/execute (fn [_ _ _ _] (js/Promise.resolve response))} + (t/is (= 1 (await (deferred/execute (transit/encode-str request))))) + (let [log (fs/readFileSync (:log-path home) "utf8") + position (str/last-index-of log (str "Comparison " (:comparison-id home) "\n")) + report (subs log (or position 0))] + (t/is (some? position)) + (t/is (str/includes? report "Failures\n Configuration \"0\":")) + (t/is (= 1 (count (re-seq #"page close failed" report)))) + (t/is (str/includes? report (get-in config [:run :record-path]))) + (t/is (str/includes? report (get-in config [:run :log-path]))) + (t/is (str/includes? report (:record-path home)))))))) + (t/deftest ^:async gpu-rejection-stops-later-matrix-configurations (doseq [[hardware-first close-fails] [[false false] [true false] [true true]]] (let [root @temporary-root