🚧 WIP 29.1, add options schema

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)})

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)

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)))

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