diff --git a/src/practitest_firecracker/api.clj b/src/practitest_firecracker/api.clj index 77ebb8e..6f239bb 100644 --- a/src/practitest_firecracker/api.clj +++ b/src/practitest_firecracker/api.clj @@ -61,12 +61,52 @@ (throw-api-exception ex-info status body uri)) (throw-api-exception ex-info status body uri)))))) +(defn parse-account-rate + "Read the account's API limit (calls per minute) from the /account.json + response body, or nil when it is missing/unparseable." + [body] + (let [api-max (get-in body [:data :attributes :api-max])] + (when (and (number? api-max) (pos? api-max)) + api-max))) + +(defn fetch-account-api-max + "One un-throttled GET to discover the account's API rate limit (calls/min). + Returns nil on any error or non-200 (e.g. a backend without this endpoint), + so callers transparently fall back to the configured rate." + [base-uri credentials] + (try + (let [{:keys [status body]} (http/get (build-uri base-uri account-uri) + {:basic-auth credentials + :throw-exceptions false + :as :json + :query-params {:source "firecracker" + :firecracker-version (get-current-version)}})] + (when (= status 200) + (parse-account-rate body))) + (catch Exception _e nil))) + +(defn effective-api-rate + "The rate FC should actually use: the lower of the configured rate and the + account's real limit. Falls back to the configured rate when the account + limit is unknown (nil)." + [configured-rate account-rate] + (if account-rate + (min configured-rate account-rate) + configured-rate)) + (defn make-client [{:keys [email api-token api-uri max-api-rate]}] - {:credentials [email api-token] - :base-uri (str api-uri - (if (string/ends-with? api-uri "/") "" "/") - "api/v2") - :max-api-rate-throttler (create-api-throttler max-api-rate)}) + (let [base-uri (str api-uri + (if (string/ends-with? api-uri "/") "" "/") + "api/v2") + credentials [email api-token] + account-rate (fetch-account-api-max base-uri credentials) + rate (effective-api-rate max-api-rate account-rate)] + (when (and account-rate (< account-rate max-api-rate)) + (log/warnf "--max-api-rate=%s exceeds account API limit %s/min; throttling at %s/min" + max-api-rate account-rate rate)) + {:credentials credentials + :base-uri base-uri + :max-api-rate-throttler (create-api-throttler rate)})) (defn ll-testset-instances [{:keys [base-uri credentials max-api-rate-throttler]} [project-id display-action-logs] testset-id test-ids] (when display-action-logs (log/infof "get instances from testsets %s" testset-id)) diff --git a/src/practitest_firecracker/const.clj b/src/practitest_firecracker/const.clj index ad69d6b..6eef796 100644 --- a/src/practitest_firecracker/const.clj +++ b/src/practitest_firecracker/const.clj @@ -13,6 +13,7 @@ (def ^:const bulk-list-tests-uri "/projects/%d/tests/bulk_search.json") (def ^:const list-testsets-uri "/projects/%d/sets.json") (def ^:const custom-field-uri "/projects/%d/custom_fields/%d.json") +(def ^:const account-uri "/account.json") ;; Used when we get testset instances for multiple test ids ;; It's a GET request, so if we pass too many test IDs, we get the "URL too long" error diff --git a/src/practitest_firecracker/core.clj b/src/practitest_firecracker/core.clj index 0543f31..8c98090 100644 --- a/src/practitest_firecracker/core.clj +++ b/src/practitest_firecracker/core.clj @@ -113,10 +113,10 @@ :scenarios-map scenarios-map :sample false) additional-reports (send-directory parsed-dirs options) - start-time (t/now) + start-time (t/now)] + - ] (-> (create-testsets client options additional-reports) (group-tests client options) diff --git a/src/practitest_firecracker/parser/core.clj b/src/practitest_firecracker/parser/core.clj index 5829750..f880444 100644 --- a/src/practitest_firecracker/parser/core.clj +++ b/src/practitest_firecracker/parser/core.clj @@ -450,11 +450,20 @@ filtered-paths (for [file filtered-files] (.getAbsolutePath file))] filtered-paths)) +(defn- parse-file + "Parse a single XML report file. Empty/whitespace-only or malformed files are + skipped with a warning so one bad file does not abort the whole run." + [path] + (let [content (slurp path)] + (if (str/blank? content) + (log/warn "Skipping empty XML report file:" path) + (try + (zip-str content) + (catch Exception e + (log/warn "Skipping unparseable XML report file:" path "-" (.getMessage e))))))) + (defn parse-files [directory] - (let [filtered-paths (get-files-path directory ".xml") - files (for [path filtered-paths] (slurp path)) - parsed-files (for [file files] (zip-str file))] - parsed-files)) + (keep parse-file (get-files-path directory ".xml"))) (defn merge-results [parsed-files {:keys [multitestset testset-name] diff --git a/src/practitest_firecracker/query_dsl.cljc b/src/practitest_firecracker/query_dsl.cljc index 08326e0..f1af42f 100644 --- a/src/practitest_firecracker/query_dsl.cljc +++ b/src/practitest_firecracker/query_dsl.cljc @@ -109,21 +109,32 @@ (string/starts-with? (str query) "?") (str "") :else (str query)))))) +(defn dsl-expression? [s] + ;; DSL values are either function calls "(...)" or field references "?...". + ;; Anything else is a static literal and must be preserved verbatim - feeding it + ;; to edn/read-string would truncate it to the first whitespace-delimited token + ;; (e.g. "In Progress" -> 'In). + (let [trimmed (string/triml s)] + (or (string/starts-with? trimmed "(") + (string/starts-with? trimmed "?")))) + (defn read-query [s] - (let [query (edn/read-string s) - compiler (fn compile-query [query] - (if (list? query) - (let [[op & args] query] - {:op (compile-query op) - :args (vec (map compile-query args))}) - query))] - (if (not (or (number? query) - (string? query) - (double? query) - (nil? query))) - (with-meta (compiler query) - {:query true}) - (compiler query)))) + (if (and (string? s) (not (dsl-expression? s))) + s + (let [query (edn/read-string s) + compiler (fn compile-query [query] + (if (list? query) + (let [[op & args] query] + {:op (compile-query op) + :args (vec (map compile-query args))}) + query))] + (if (not (or (number? query) + (string? query) + (double? query) + (nil? query))) + (with-meta (compiler query) + {:query true}) + (compiler query))))) (defn try-read-query [s] #?(:cljs diff --git a/test/practitest_firecracker/api_test.clj b/test/practitest_firecracker/api_test.clj new file mode 100644 index 0000000..17ccf24 --- /dev/null +++ b/test/practitest_firecracker/api_test.clj @@ -0,0 +1,21 @@ +(ns practitest-firecracker.api-test + (:require [clojure.test :refer :all] + [practitest-firecracker.api :as api])) + +(deftest effective-api-rate-test + (testing "clamps to the account limit when the configured rate is higher" + (is (= 30 (api/effective-api-rate 300 30)))) + (testing "keeps the configured rate when it is at or below the account limit" + (is (= 30 (api/effective-api-rate 30 300))) + (is (= 30 (api/effective-api-rate 30 30)))) + (testing "falls back to the configured rate when the account limit is unknown" + (is (= 300 (api/effective-api-rate 300 nil))))) + +(deftest parse-account-rate-test + (testing "reads calls-per-minute (api-max) from the account.json envelope" + (is (= 30 (api/parse-account-rate {:data {:attributes {:api-max 30}}}))) + (is (= 120 (api/parse-account-rate {:data {:attributes {:api-max 120}}})))) + (testing "returns nil when api-max is missing or invalid" + (is (nil? (api/parse-account-rate {:data {:attributes {}}}))) + (is (nil? (api/parse-account-rate {}))) + (is (nil? (api/parse-account-rate {:data {:attributes {:api-max 0}}}))))) diff --git a/test/practitest_firecracker/parser/core_test.clj b/test/practitest_firecracker/parser/core_test.clj new file mode 100644 index 0000000..e4189b1 --- /dev/null +++ b/test/practitest_firecracker/parser/core_test.clj @@ -0,0 +1,28 @@ +(ns practitest-firecracker.parser.core-test + (:require [clojure.test :refer :all] + [clojure.java.io :as io] + [practitest-firecracker.parser.core :as parser])) + +(defn- write-file! [dir name content] + (let [f (io/file dir name)] + (spit f content) + f)) + +(deftest parse-files-skips-empty-and-unparseable + (testing "empty, whitespace-only and malformed XML files are skipped, valid ones parsed" + (let [dir (io/file (System/getProperty "java.io.tmpdir") + (str "fc-parse-files-" (System/currentTimeMillis)))] + (.mkdirs dir) + (try + (write-file! dir "empty.xml" "") + (write-file! dir "blank.xml" " \n\t ") + (write-file! dir "broken.xml" "") + (write-file! dir "good.xml" + "") + (let [parsed (doall (parser/parse-files dir))] + ;; Only the single valid file should survive + (is (= 1 (count parsed))) + (is (= :testsuite (:tag (first (first parsed)))))) + (finally + (doseq [f (.listFiles dir)] (.delete f)) + (.delete dir)))))) diff --git a/test/practitest_firecracker/query_dsl_test.clj b/test/practitest_firecracker/query_dsl_test.clj index 29ea6f8..ae8c51d 100644 --- a/test/practitest_firecracker/query_dsl_test.clj +++ b/test/practitest_firecracker/query_dsl_test.clj @@ -101,4 +101,17 @@ (= (read-query "(get 1 [\"val\"])") {:op 'get - :args [1 ["val"]]})))) \ No newline at end of file + :args [1 ["val"]]})))) + +(deftest test-read-query-static-values + (testing "multi-word static value is preserved verbatim, not truncated to the first word" + (is (= "In Progress" (read-query "In Progress"))) + (is (= "In Progress" (eval-query {:name "x"} (read-query "In Progress"))))) + (testing "static value is not flagged as a query, so additional fields pass it through unchanged" + (is (not (query? (read-query "In Progress"))))) + (testing "single-word static value still works" + (is (= "Regression" (eval-query {:name "x"} (read-query "Regression"))))) + (testing "DSL function calls and field references still parse as queries" + (is (= {:op 'get :args [1 ["val"]]} (read-query "(get 1 [\"val\"])"))) + (is (query? (read-query "(get 1 [\"val\"])"))) + (is (query? (read-query "?package-name"))))) \ No newline at end of file