diff --git a/frontend/performance/benches/render_wasm/node/commands.cljs b/frontend/performance/benches/render_wasm/node/commands.cljs index 704d8cffd1..be9d1fdc97 100644 --- a/frontend/performance/benches/render_wasm/node/commands.cljs +++ b/frontend/performance/benches/render_wasm/node/commands.cljs @@ -7,6 +7,7 @@ (ns benches.render-wasm.node.commands "Execution commands loaded after CLI validation." (:require + [app.common.schema :as sm] [app.common.transit :as transit] [benches.render-wasm.node.artifacts :as artifacts] [benches.render-wasm.node.build :as build] @@ -30,7 +31,7 @@ (ex-info "run interrupted" {:type ::interrupted :signal name})))))] (try - (await (run/run root options signal)) + (await (run/run root (select-keys options (sm/keys run/schema:run-options)) signal)) (catch :default cause {:error cause :log-path (:log-path (ex-data cause)) :exit-code (case @interrupted "SIGINT" 130 "SIGTERM" 143 1)}) diff --git a/frontend/performance/benches/render_wasm/node/feature_comparison.cljs b/frontend/performance/benches/render_wasm/node/feature_comparison.cljs index d6a0c76b79..774fb03c55 100644 --- a/frontend/performance/benches/render_wasm/node/feature_comparison.cljs +++ b/frontend/performance/benches/render_wasm/node/feature_comparison.cljs @@ -10,6 +10,7 @@ ["fs" :as fs] ["node:timers/promises" :as timers] ["path" :as path] + [app.common.schema :as sm] [benches.render-wasm.failures :as failures] [benches.render-wasm.node.artifacts :as artifacts] [benches.render-wasm.node.build :as build] @@ -401,8 +402,7 @@ {:home ((:create-run-home services) {:root root :run-id run-id - :output-dir (path/join (:dir home) "configs" slug) - :options options})} + :output-dir (path/join (:dir home) "configs" slug)})} (catch :default cause (if (storage-error? cause) (throw cause) @@ -427,7 +427,7 @@ (try (await ((:execute-run services) root run-home selected cases - (assoc options :build-id (:id selected)) + (select-keys options (sm/keys run/schema:run-options)) signal)) (catch :default cause (if (storage-error? cause) diff --git a/frontend/performance/benches/render_wasm/node/run.cljs b/frontend/performance/benches/render_wasm/node/run.cljs index 60694d8f68..0e449433dc 100644 --- a/frontend/performance/benches/render_wasm/node/run.cljs +++ b/frontend/performance/benches/render_wasm/node/run.cljs @@ -35,6 +35,31 @@ [benches.render-wasm.result :as result] [clojure.string :as str])) +(def schema:run-options + "Resolved options for run preparation and execution." + (let [timeout-ms [:int {:min 1 :max 2147483647}]] + [:map {:closed true} + [:seed [:int {:min 0 :max 4294967295}]] + [:warmups [:int {:min 0 :max js/Number.MAX_SAFE_INTEGER}]] + [:repetitions [:int {:min 1 :max js/Number.MAX_SAFE_INTEGER}]] + [:timeout-ms timeout-ms] + [:build-timeout-ms timeout-ms] + [:startup-timeout-ms timeout-ms] + [:filter {:optional true} [:maybe :string]] + [:build-id {:optional true} [:string {:min 1}]] + [:output-dir {:optional true} [:maybe [:string {:min 1}]]] + [:default-features {:optional true} :boolean] + [:features {:optional true} [:vector [:string {:min 1}]]] + [:screenshots {:optional true} :boolean] + [:remove {:optional true} :boolean] + [:require-gpu {:optional true} :boolean] + [:browser {:optional true} [:map-of :keyword fn?]]])) + +(def ^:private check-options + (sm/check-fn schema:run-options + :hint "invalid run options" + :type ::invalid-options)) + (defn- now [] (.toISOString (js/Date.))) @@ -145,19 +170,19 @@ (defn- record-screenshot-skip [state case-id reason] - (when (:screenshots? (:opts state)) + (when (:screenshots (:opts state)) (when-not (screenshot-status state case-id) (record-screenshot-status state case-id {:status :skipped :reason reason})))) (defn- ensure-screenshot-status [state case reason] - (when (:screenshots? (:opts state)) + (when (:screenshots (:opts state)) (when-not (screenshot-status state (:id case)) (record-screenshot-skip state (:id case) reason)))) (defn- ^:async screenshot-case [state case page] - (when (:screenshots? (:opts state)) + (when (:screenshots (:opts state)) (let [{:keys [file path]} (runs/screenshot-path (:home state) (:id case)) opts (:opts state) bounds {:deadline (process/deadline (:startup-timeout-ms opts)) @@ -276,7 +301,7 @@ (false? (:software graphics)) (usable-graphics-identity? (:vendor graphics)) (usable-graphics-identity? (:renderer graphics)))] - (if (and (:require-gpu? (:opts state)) (not accepted)) + (if (and (:require-gpu (:opts state)) (not accepted)) (do (record-failure state (:case-id preparation) :gpu {:status "failed" :phase "gpu" @@ -563,7 +588,7 @@ [state] (let [termination (get-termination @(:run-ref state) (interruption (:signal state)) @(:harness-failed? state))] - (when (:screenshots? (:opts state)) + (when (:screenshots (:opts state)) (swap! (:run-ref state) update-in [:metadata :diagnostics :screenshots] complete-screenshots (:cases state))) (swap! (:run-ref state) result/finish-run (now) termination) @@ -637,29 +662,18 @@ :timeout-ms (:timeout-ms opts) :build-timeout-ms (:build-timeout-ms opts) :startup-timeout-ms (:startup-timeout-ms opts) - :screenshots? (boolean (:screenshots? opts)) - :require-gpu? (boolean (:require-gpu? opts)) + :screenshots? (boolean (:screenshots opts)) + :require-gpu? (boolean (:require-gpu opts)) :penpot-gpu (or (.-PENPOT_GPU (.-env js/process)) nil) :runner-git {:sha nil :dirty nil} :runtime-helper-identity :unverified :screenshots {} :failures []}}) -(defn- normalize-options - [options] - (-> options - (assoc :screenshots? (boolean (or (:screenshots? options) - (:screenshots options))) - :remove? (boolean (or (:remove? options) - (:remove options))) - :require-gpu? (boolean (or (:require-gpu? options) - (:require-gpu options)))) - (dissoc :screenshots :remove :require-gpu))) - (defn- create-run-record - [root selected-build cases opts] + [root run-id selected-build cases opts] (result/create-run - {:run-id (:run-id opts) + {:run-id run-id :started-at (now) :plan {:cases cases :warmups (:warmups opts) @@ -668,23 +682,22 @@ :metadata (metadata root selected-build opts)})) (defn- create-run-state - [root home selected-build cases options signal] - (let [opts (assoc (normalize-options options) :run-id (:run-id home))] - {:root root - :selected-build selected-build - :cases cases - :opts opts - :signal signal - :home home - :run-ref (atom (create-run-record root selected-build cases opts)) - :storage-error-ref (atom nil) - :owner-ref (atom nil) - :page-ref (atom nil) - :host-ref (atom nil) - :launch-deadline (atom nil) - :abort-run? (atom false) - :harness-failed? (atom false) - :services (merge browser/service (:browser opts))})) + [root home selected-build cases opts signal] + {:root root + :selected-build selected-build + :cases cases + :opts opts + :signal signal + :home home + :run-ref (atom (create-run-record root (:run-id home) selected-build cases opts)) + :storage-error-ref (atom nil) + :owner-ref (atom nil) + :page-ref (atom nil) + :host-ref (atom nil) + :launch-deadline (atom nil) + :abort-run? (atom false) + :harness-failed? (atom false) + :services (merge browser/service (:browser opts))}) (defn- record-session-failure [state stage cause] @@ -719,7 +732,7 @@ (defn- remove-run-build [state saved?] - (when (and saved? (:remove? (:opts state))) + (when (and saved? (:remove (:opts state))) (try (remove-build (:root state) (:selected-build state)) nil @@ -772,7 +785,7 @@ - `home`: the reserved output home. - `selected-build`: the prepared build. - `cases`: the selected cases. - - `options`: resolved CLI options + - `options`: resolved options satisfying `schema:run-options`. - `signal`: an optional abort signal. A browser startup failure leaves a valid partial record. @@ -781,6 +794,7 @@ storage-error. A storage error reports any failed record or log write, including a failure followed by a successful write." [root home selected-build cases options signal] + (check-options options) (binding [log/*log-path* (:log-path home)] (let [state (create-run-state root home selected-build cases options signal)] (persist state) @@ -824,17 +838,16 @@ (defn ^:async run "Prepares or loads a build, collects matching cases and executes one run. - A build or catalog failure rethrows with build facts without writing to the - build output home + - `root`: the frontend checkout root. + - `options`: resolved options satisfying `schema:run-options`. + - `signal`: an optional abort signal. - - root: The root of the frontend code checkout - - options: see `schema:run-options` - - signal: - " + Validates options before allocating output. Returns the execution result. + A build or catalog failure throws with the run log path and build facts." [root options signal] - (let [opts (normalize-options options) + (let [opts (check-options options) home (runs/create-home {:root root :run-id (str (random-uuid)) - :output-dir (:output-dir opts) :remove (:remove? opts)})] + :output-dir (:output-dir opts) :remove (:remove opts)})] (binding [log/*log-path* (:log-path home)] (try (log/result (str "Run " (:run-id home))) diff --git a/frontend/test/performance_tests/render_wasm/run_test.cljs b/frontend/test/performance_tests/render_wasm/run_test.cljs index c2c1e017a1..7e4ee275f2 100644 --- a/frontend/test/performance_tests/render_wasm/run_test.cljs +++ b/frontend/test/performance_tests/render_wasm/run_test.cljs @@ -85,8 +85,8 @@ (defn- fake-browser [{:keys [launch-failure new-page-failure close-page-failure prepare-failure run-failure screenshot-failure - interruption controller effective-graphics] - :or {effective-graphics (constantly {})}}] + interruption controller effective-graphics metrics] + :or {effective-graphics (constantly {}) metrics {}}}] (let [state (atom {:launches 0 :pages 0 :close-pages 0 :closes 0 :host nil :runs {} :preparations {}})] {:state state @@ -163,7 +163,7 @@ :response {:status "failed" :phase "interact" :message "attempt rejected"} - {:status "ok" :metrics {} :trace {}}))) + {:status "ok" :metrics metrics :trace {}}))) :screenshot (fn [page file _] (check-page page) @@ -173,8 +173,7 @@ (defn- options [fake out] - {:run-id "test-run" - :output-dir out + {:output-dir out :seed 42 :filter nil :warmups 0 @@ -191,7 +190,7 @@ (execute fake cases opts (js/AbortController.))) ([_fake cases opts controller] (run/execute (artifacts/frontend-root) - (runs/create-home {:root (artifacts/frontend-root) :run-id (:run-id opts) + (runs/create-home {:root (artifacts/frontend-root) :run-id "test-run" :output-dir (:output-dir opts)}) (selected-build (build-dir) "test-build") cases opts (.-signal controller)))) @@ -208,6 +207,45 @@ [result] (result/decode (fs/readFileSync (get-in result [:home :record-path]) "utf8"))) +(t/deftest ^:async run-rejects-invalid-options-before-allocating-output + (doseq [invalid [{:seed 4294967296} + {:warmups -1} + {:repetitions 1.5} + {:timeout-ms js/Infinity} + {:build-timeout-ms 2147483648} + {:startup-timeout-ms 0} + {:screenshots "false"} + {:remove nil} + {:require-gpu 1} + {:repetitons 1}]] + (let [out (output-dir) + opts (merge (dissoc (options nil out) :browser) + {:build-id "unavailable"} invalid) + cause (try + (await (run/run @temporary-root opts nil)) + nil + (catch :default cause cause))] + (t/testing (pr-str invalid) + (t/is (= ::run/invalid-options (:type (ex-data cause)))) + (t/is (not (fs/existsSync out))))))) + +(t/deftest ^:async execute-rejects-invalid-options-before-writing-evidence + (let [root @temporary-root + fake (fake-browser {}) + home (runs/create-home {:root root :run-id "invalid-options" + :output-dir (output-dir)}) + files (vec (fs/readdirSync (:dir home))) + log-text (fs/readFileSync (:log-path home) "utf8") + opts (assoc (options fake (:dir home)) :screenshots "false") + cause (try + (await (run/execute root home (selected-build (build-dir) "test-build") + [(collected :case/a :fresh)] opts nil)) + nil + (catch :default cause cause))] + (t/is (= ::run/invalid-options (:type (ex-data cause)))) + (t/is (= files (vec (fs/readdirSync (:dir home))))) + (t/is (= log-text (fs/readFileSync (:log-path home) "utf8"))))) + (t/deftest ^:async retained-run-prints-results-and-keeps-all-levels-in-its-log (doseq [level [:info :warn :debug]] (let [root @temporary-root @@ -801,7 +839,7 @@ opts (options fake (output-dir)) build (cond-> (selected-build (build-dir) "test-build") (not= ::missing environment) (assoc-in [:manifest :environment] environment)) - home (runs/create-home {:root root :run-id (:run-id opts) :output-dir (:output-dir opts)}) + home (runs/create-home {:root root :run-id "test-run" :output-dir (:output-dir opts)}) controller (js/AbortController.) executed (await (run/execute root home build [(collected :case/a :fresh)] opts (.-signal controller))) @@ -825,7 +863,7 @@ :output-dir out :remove true}) result (await (run/execute root home (selected-build dir id) [(collected :case/a :fresh)] - (assoc (options fake out) :remove? true) + (assoc (options fake out) :remove true) (.-signal (js/AbortController.))))] (t/is (fs/existsSync (get-in result [:home :record-path]))) (t/is (fs/existsSync dir)) @@ -878,6 +916,36 @@ (on-created build) (js/Promise.resolve build))) +(t/deftest ^:async feature-comparison-completes-with-managed-output + (let [root @temporary-root + cases [(collected :case/a :fresh)] + fake (fake-browser {:metrics {:first-render-ms + (result/metric {:value 1 :unit :ms + :semantics :prepared-scene-to-full + :basis {:kind :attempt-total} + :zero-policy :positive})}}) + prepared {:plan (feature-plan/plan ["a"] {:dry-run true}) + :cases cases + :output-dir nil + :options (dissoc (options nil nil) :browser)} + 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))}})) + saved (result/decode (fs/readFileSync (get-in response [:home :record-path]) "utf8"))] + (t/is (= 0 (:exit-code response))) + (t/is (= [:completed :completed] (mapv :status (:configurations saved)))) + (doseq [configuration (:configurations saved)] + (let [run (result/decode (fs/readFileSync (get-in configuration [:run :record-path]) "utf8"))] + (t/is (= :completed (get-in run [:termination :reason]))) + (t/is (= 1 (count (:attempts run)))))))) + (t/deftest ^:async gpu-rejection-stops-later-matrix-configurations (doseq [[hardware-first close-fails] [[false false] [true false] [true true]]] (let [root @temporary-root @@ -887,7 +955,7 @@ prepared {:plan (feature-plan/plan ["a"] {:dry-run true}) :cases cases :output-dir out - :options (assoc (dissoc (options nil out) :run-id :browser) + :options (assoc (dissoc (options nil out) :browser) :require-gpu true :repetitions 2)} response (await (feature-comparison/execute @@ -927,7 +995,7 @@ prepared {:plan (feature-plan/plan ["a"] {:dry-run true}) :cases cases :output-dir out - :options (dissoc (options nil out) :run-id :browser)} + :options (dissoc (options nil out) :browser)} response (await (feature-comparison/execute root prepared nil