From c6c4b78bb170efa3fd52597c5f2476fe8b3b1088 Mon Sep 17 00:00:00 2001 From: Miguel de Benito Delgado Date: Sat, 31 Oct 2026 14:05:26 +0100 Subject: [PATCH] :recycle: Consolidate benchmark output dir creation --- .../render_wasm/node/feature_comparison.cljs | 12 ++-- .../render_wasm/node/feature_plan.cljs | 4 +- .../benches/render_wasm/node/files.cljs | 65 +++++++++++++++---- .../benches/render_wasm/node/runs.cljs | 24 +------ .../render_wasm/feature_comparison_test.cljs | 2 +- .../render_wasm/runs_test.cljs | 22 ++++++- 6 files changed, 83 insertions(+), 46 deletions(-) diff --git a/frontend/performance/benches/render_wasm/node/feature_comparison.cljs b/frontend/performance/benches/render_wasm/node/feature_comparison.cljs index 4dcdfed6e1..948c3b5efd 100644 --- a/frontend/performance/benches/render_wasm/node/feature_comparison.cljs +++ b/frontend/performance/benches/render_wasm/node/feature_comparison.cljs @@ -43,18 +43,14 @@ An explicit directory resolves from the caller's cwd and must not exist. Paths inside the managed builds tree are rejected." [root comparison-id output-dir] - (files/check-id comparison-id) + (files/check-id comparison-id "invalid feature comparison id") (let [default-dir (path/join (artifacts/managed-root root) "feature-comparisons" comparison-id) - dir (runs/check-output-dir (or output-dir default-dir))] + dir (files/check-output-dir (or output-dir default-dir))] (when (artifacts/inside-builds? root dir) (files/invalid-home "feature comparison cannot use a directory inside builds" {:path dir})) - (try - (files/create-home dir "feature-comparison") - (catch :default cause - (throw (ex-info "cannot create feature comparison home" - {:type ::invalid-home :path dir :cause (errors/message-of cause)} - cause)))))) + (files/create-home dir "feature-comparison" + "cannot create feature comparison home"))) (defn write-record "Validates schema:feature-comparison and atomically replaces the saved file." diff --git a/frontend/performance/benches/render_wasm/node/feature_plan.cljs b/frontend/performance/benches/render_wasm/node/feature_plan.cljs index 7b41b839c9..8b34f254e8 100644 --- a/frontend/performance/benches/render_wasm/node/feature_plan.cljs +++ b/frontend/performance/benches/render_wasm/node/feature_plan.cljs @@ -11,9 +11,9 @@ ["path" :as path] [benches.render-wasm.cases :as cases] [benches.render-wasm.feature-plan :as feature-plan] + [benches.render-wasm.node.files :as files] [benches.render-wasm.node.options :as options] [benches.render-wasm.node.process :as process] - [benches.render-wasm.node.runs :as runs] [clojure.string :as str])) (defn- get-root-features @@ -41,7 +41,7 @@ opts {:default-features false}) plan (feature-plan/plan (:features opts) opts) - output-dir (runs/check-output-dir (:output-dir opts)) + output-dir (files/check-output-dir (:output-dir opts)) cwd (path/resolve root ".." "render-wasm") metadata (await (process/run {:command "cargo" diff --git a/frontend/performance/benches/render_wasm/node/files.cljs b/frontend/performance/benches/render_wasm/node/files.cljs index 551581ba78..b6bfc983bb 100644 --- a/frontend/performance/benches/render_wasm/node/files.cljs +++ b/frontend/performance/benches/render_wasm/node/files.cljs @@ -9,6 +9,7 @@ (:require ["fs" :as fs] ["path" :as path] + [benches.render-wasm.errors :as errors] [clojure.string :as str])) (defn valid-id? @@ -26,9 +27,40 @@ (not= relative "..") (not (str/starts-with? relative (str ".." path/sep)))))) -(defn create-home - "Creates `dir` and its parents, reserves an empty log, and returns the - directory, log path and record path. `stem` names the .log and .transit files." +(defn invalid-home + "Throws an ::invalid-home exception with `data` and an optional cause." + ([message data] + (throw (ex-info message (assoc data :type ::invalid-home)))) + ([message data cause] + (throw (ex-info message (assoc data :type ::invalid-home) cause)))) + +(defn check-id + "Returns `id` when it is valid, or throws ::invalid-home with `message`." + [id message] + (when-not (valid-id? id) + (invalid-home message {:id id})) + id) + +(defn check-output-dir + "Resolves an explicit user directory from the caller's cwd and checks that it + does not exist. Returns nil for nil. Does not create anything." + [dir] + (when (some? dir) + (when (str/blank? dir) + (invalid-home "output directory requires a nonempty path" {})) + (let [resolved (path/resolve dir) + exists? (try + (fs/lstatSync resolved #js {:throwIfNoEntry false}) + (catch :default cause + (let [detail (errors/message-of cause)] + (invalid-home (str "cannot inspect output directory: " detail) + {:path resolved :cause detail} + cause))))] + (when exists? + (invalid-home "output directory already exists" {:path resolved})) + resolved))) + +(defn- create-home* [dir stem] (let [log-path (path/join dir (str stem ".log"))] (fs/mkdirSync dir #js {:recursive true}) @@ -37,15 +69,26 @@ :log-path log-path :record-path (path/join dir (str stem ".transit"))})) -(defn invalid-home - [message data] - (throw (ex-info message (assoc data :type ::invalid-home)))) +(defn create-home + "Creates `dir` and its parents, reserves an empty log. -(defn check-id - [id] - (when-not (valid-id? id) - (invalid-home "invalid id" {:id id})) - id) + A filesystem failure throws ::invalid-home with `message` and `:path`, and + keeps the original cause. + + Args: + dir: path to create. + stem: names the .log and .transit files. + message: used for errors + + Returns: + Directory created, log path and record path." + [dir stem message] + (try + (create-home* dir stem) + (catch :default cause + (invalid-home message + {:path dir :cause (errors/message-of cause)} + cause)))) (defn get-real-path "Resolves symlink ancestors even when the final path does not exist yet." diff --git a/frontend/performance/benches/render_wasm/node/runs.cljs b/frontend/performance/benches/render_wasm/node/runs.cljs index 408a53ece3..638ee35f20 100644 --- a/frontend/performance/benches/render_wasm/node/runs.cljs +++ b/frontend/performance/benches/render_wasm/node/runs.cljs @@ -14,40 +14,22 @@ (:require ["fs" :as fs] ["path" :as path] - [benches.render-wasm.errors :as errors] [benches.render-wasm.node.artifacts :as artifacts] [benches.render-wasm.node.files :as files] [benches.render-wasm.result :as result] [clojure.string :as str])) -(defn check-output-dir - "Resolves an explicit user directory from the caller's cwd and checks that it - does not exist. Nil selects the managed default. Does not create anything." - [dir] - (when (some? dir) - (when (str/blank? dir) - (files/invalid-home "output directory requires a nonempty path" {})) - (let [resolved (path/resolve dir)] - (when (fs/lstatSync resolved #js {:throwIfNoEntry false}) - (files/invalid-home "output directory already exists" {:path resolved})) - resolved))) - (defn create-home "Creates one run home and its empty log. The record is written separately after the plan exists. An explicit directory must not exist; a managed home uses the frontend root regardless of cwd." [{:keys [root run-id output-dir remove]}] - (files/check-id run-id) - (let [dir (check-output-dir + (files/check-id run-id "invalid run id") + (let [dir (files/check-output-dir (or output-dir (path/join (artifacts/managed-root root) "runs" run-id)))] (when (and remove (artifacts/inside-builds? root dir)) (files/invalid-home "--remove cannot use an output directory inside builds" {:path dir})) - (try - (assoc (files/create-home dir "run") :run-id run-id) - (catch :default cause - (throw (ex-info "cannot create run home" - {:type ::invalid-home :path dir :cause (errors/message-of cause)} - cause)))))) + (assoc (files/create-home dir "run" "cannot create run home") :run-id run-id))) (defn get-record "Reads and validates a saved run record from a path relative to cwd." 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 b7dd904586..95806ab898 100644 --- a/frontend/test/performance_tests/render_wasm/feature_comparison_test.cljs +++ b/frontend/test/performance_tests/render_wasm/feature_comparison_test.cljs @@ -783,7 +783,7 @@ (:dir home))) (t/is (= "" (fs/readFileSync (:log-path home) "utf8")))) (doseq [id [nil "" "../escape" "a/b" "a\\b"]] - (t/is (thrown-with-msg? js/Error #"invalid id" + (t/is (thrown-with-msg? js/Error #"invalid feature comparison id" (feature-comparison/create-home (frontend-root) id nil)))) (let [existing (path/join @temporary-root "existing")] (fs/mkdirSync existing) diff --git a/frontend/test/performance_tests/render_wasm/runs_test.cljs b/frontend/test/performance_tests/render_wasm/runs_test.cljs index 97c45bea94..efbe3a0b6a 100644 --- a/frontend/test/performance_tests/render_wasm/runs_test.cljs +++ b/frontend/test/performance_tests/render_wasm/runs_test.cljs @@ -96,10 +96,10 @@ (t/deftest malformed-ids-and-empty-paths-fail-at-the-home-boundary (doseq [id ["../escape" "" "a/b" "a\\b"]] - (t/is (thrown-with-msg? js/Error #"invalid id" + (t/is (thrown-with-msg? js/Error #"invalid run id" (runs/create-home {:root @temporary-root :run-id id})))) (t/is (thrown-with-msg? js/Error #"nonempty path" - (runs/check-output-dir " ")))) + (files/check-output-dir " ")))) (t/deftest home-creation-reports-an-invalid-parent (let [parent (path/join @temporary-root "file") @@ -107,7 +107,23 @@ (fs/writeFileSync parent "occupied") (let [error (failure #(runs/create-home {:root @temporary-root :run-id "run-6" :output-dir dir}))] - (t/is (= "ENOTDIR" (.-code (or (ex-cause error) error))))))) + (t/is (= ::files/invalid-home (:type (ex-data error)))) + (t/is (= (path/resolve dir) (:path (ex-data error)))) + (t/is (= "ENOTDIR" (.-code (ex-cause error)))) + (t/is (= (ex-message (ex-cause error)) (:cause (ex-data error)))) + (t/is (str/includes? (ex-message error) "cannot inspect output directory")) + (t/is (str/includes? (ex-message error) "ENOTDIR"))))) + +(t/deftest home-creation-wraps-log-failures + (let [dir (path/join @temporary-root "occupied-log")] + (fs/mkdirSync dir) + (fs/writeFileSync (path/join dir "run.log") "") + (let [error (failure #(files/create-home dir "run" "cannot create run home"))] + (t/is (= ::files/invalid-home (:type (ex-data error)))) + (t/is (= "cannot create run home" (ex-message error))) + (t/is (= dir (:path (ex-data error)))) + (t/is (= "EEXIST" (.-code (ex-cause error)))) + (t/is (= (ex-message (ex-cause error)) (:cause (ex-data error))))))) (t/deftest explicit-home-creates-missing-parents (let [out (path/join @temporary-root "missing" "parents" "run")