ci: rework unit tests (clj-commons#185)

Treat tests to run as data. Have GitHub Actions source its matrix from that data. This allows us more flexibility in adapting to changes (ex. shadow-cljs now requires >= jdk11) It greatly breaks down the granularity of tests. Although we might have gone a bit too far here? We'll see how elapsed ci times are affected and adapt as necessary. Although we are running many more GitHub Actions jobs, we are running fewer tests. For example, cljs tests are now only run under a single JDK instead of all JDKs. Linting tests are run under all oses (to make sure they will work for contributors, regardless of os) but under only a single JDK version and Clojure version. Also: I avoided the complexity of optimizing setup. Ex. we rarely need Planck but always install it. We only need npm setup for cljs but always set it up. Can add in that complexity at a later date if there is merit.
lread · Sep 13, 2022 · 8899b1b · 8899b1b
1 parent 4e73fbf
commit 8899b1b
Show file tree

Hide file tree

Showing 6 changed files with 191 additions and 50 deletions.
diff --git a/.github/workflows/unit-test.yml b/.github/workflows/unit-test.yml
@@ -1,17 +1,56 @@
 name: Unit Tests
 
-on: [push, pull_request]
+on:
+  push:
+    branches: ['main']
+  pull_request:
 
 jobs:
+  setup:
+    runs-on: ubuntu-latest
+
+    outputs:
+      tests: ${{ steps.set-tests.outputs.tests }}
+
+    steps:
+    - uses: actions/checkout@v3
+
+    - name: Clojure deps cache
+      uses: actions/cache@v3
+      with:
+        path: |
+          ~/.m2/repository
+          ~/.deps.clj
+          ~/.gitlibs
+        key: cljdeps-${{ hashFiles('deps.edn', 'bb.edn') }}
+        restore-keys: cljdeps-
+
+    - name: "Setup Java"
+      uses: actions/setup-java@v3
+      with:
+        distribution: 'temurin'
+        java-version: '11'
+
+    - name: Install Clojure Tools
+      uses: DeLaGuardo/[email protected]
+      with:
+        cli: 'latest'
+        bb: 'latest'
+
+    - id: set-tests
+      name: Set test var for matrix
+      # run test.clj directly instead of via bb task to avoid generic task output
+      run: echo "::set-output name=tests::$(bb script/ci_unit_tests.clj matrix-for-ci --format=json)"
+
   build:
+    needs: setup
     runs-on: ${{ matrix.os }}-latest
     strategy:
       fail-fast: false
       matrix:
-        os: [ windows, ubuntu, macos ]
-        java: [ '8', '11', '17' ]
+        include: ${{fromJSON(needs.setup.outputs.tests)}}
 
-    name: ${{ matrix.os }},jdk ${{ matrix.java }}
+    name: ${{ matrix.desc }}
 
     steps:
     #
@@ -49,7 +88,7 @@ jobs:
       uses: actions/setup-java@v3
       with:
         distribution: 'temurin'
-        java-version: ${{ matrix.java }}
+        java-version: ${{ matrix.jdk }}
 
     #
     # Install Planck
@@ -63,9 +102,6 @@ jobs:
         sudo apt-get update
         sudo apt-get install -y planck
       if: matrix.os == 'ubuntu'
-    - name: Install planck (macOS)
-      run: brew install planck
-      if: matrix.os == 'macos'
 
     #
     # Install Babashka
@@ -133,5 +169,5 @@ jobs:
     #
     # Run tests
     #
-    - name: Run CI tests
-      run: bb ci-unit-tests
+    - name: Run Tests
+      run: ${{ matrix.cmd }}
diff --git a/bb.edn b/bb.edn
@@ -1,4 +1,4 @@
-{:min-bb-version "0.3.7"
+{:min-bb-version "0.9.162"
  :paths ["script" "build"]
  :deps {org.clojure/data.zip {:mvn/version "1.0.0"}
         io.aviso/pretty {:mvn/version "1.1.1"}
@@ -34,5 +34,5 @@
          doc-api-diffs     {:task doc-api-diffs/-main     :doc "generate diff docs for rewrite-clj* APIs"}
          doc-update-readme {:task doc-update-readme/-main :doc "honour our contributors in README"}
          cljdoc-preview    {:task cljdoc-preview/-main    :doc "preview what docs will look like on cljdoc, use --help for args"}
-         ci-unit-tests     {:task ci-unit-tests/-main}
+         ci-unit-tests     {:task ci-unit-tests/-main     :doc "run/list continuous integration unit tests, use --help for args"}
          ci-release        {:task ci-release/-main        :doc "release tasks, use --help for args"}}}
diff --git a/doc/02-developer-guide.adoc b/doc/02-developer-guide.adoc
@@ -15,7 +15,7 @@ We test that rewrite-clj operates as expected when natively compile via GraalVM.
 Automated testing is setup using GraalVM v22 JDK11.
 
 == Prerequisites
-* Java JDK 1.8 or above
+* Java JDK 1.8 or above (shadow-cljs tests require min of JDK 11)
 * NodeJs v12 or above
 * Clojure v1.10.1.697 or above for `clojure` command
 ** Note that rewrite-clj v1 itself supports Clojure v1.8 and above
@@ -210,11 +210,22 @@ This generates tests for doc code blocks and then runs them under Clojure and Cl
 Before pushing, you likely want to mimic what is run on each push via GitHub Actions.
 
 === Unit tests
-Unit tests are run via:
+Unit tests can be run locally on your dev box.
+Some tests require a specific os and jdk, you will see warnings if a test is skipped for your current environment.
 ----
 bb ci-unit-tests
 ----
 
+Unit tests for our ci matrix are driven by:
+----
+bb script/ci_unit_tests.clj matrix-for-ci --format=json
+----
+
+To get a human readable version of the ci matrix:
+----
+bb ci-unit-tests matrix-for-ci
+----
+
 === Native image tests
 We also verify that rewrite-clj functions as expected when compiled via Graal's `native-image`.
 

diff --git a/script/ci_unit_tests.clj b/script/ci_unit_tests.clj
@@ -1,52 +1,125 @@
 #!/usr/bin/env bb
 
 (ns ci-unit-tests
-  (:require [helper.fs :as fs]
+  (:require [cheshire.core :as json]
+            [clojure.string :as string]
+            [doric.core :as doric]
+            [helper.fs :as fs]
+            [helper.jdk :as jdk]
             [helper.main :as main]
             [helper.os :as os]
             [helper.shell :as shell]
             [lread.status-line :as status]))
 
-(defn clean []
-  (doseq [dir ["target" ".cpcache" ".shadow-cljs"]]
-    (fs/delete-file-recursively dir true)))
+(defn- matrix-os []
+  (case (os/get-os)
+    :win "windows"
+    :mac "macos"
+    ;; else assume ubuntu
+    "ubuntu"))
 
-(defn lint []
-  (shell/command "bb lint"))
+;; matrix params to be used on ci
+(def ^:private all-oses ["ubuntu" "macos" "windows"])
+(def ^:private all-jdks ["8" "11" "17"])
 
-(defn check-import-vars []
-  (shell/command "bb apply-import-vars check"))
+(defn- test-tasks []
+  (concat [;; run lintish tasks across all oses to verify that they will work for all devs regardless of their os choice
+           {:desc "import-vars" :cmd "bb apply-import-vars check" :oses all-oses :jdks ["8"]}
+           {:desc "lint"        :cmd "bb lint"                    :oses all-oses :jdks ["8"]}
+           ;; test-docs on default clojure version across all oses and jdks
+           {:desc "test-doc"          :cmd "bb test-doc"          :oses all-oses :jdks all-jdks}]
+          (for [version ["1.8" "1.9" "1.10" "1.11"]]
+            {:desc (str "clj-" version)
+             :cmd (str "bb test-clj --clojure-version " version)
+             :oses all-oses
+             :jdks all-jdks})
+          ;; I'm not sure there's much value testing across jdks for ClojureScript tests, for now we'll stick with jdk 8 only
+          (for [env [{:param "node" :desc "node"}
+                     {:param "chrome-headless" :desc "browser"}]
+                opt [{:param "none"}
+                     {:param "advanced" :desc "adv"}]]
+            {:desc (str "cljs-"
+                        (:desc env)
+                        (when (:desc opt) (str "-" (:desc opt))))
+             :cmd (str "bb test-cljs --env " (:param env) " --optimizations " (:param opt))
+             :oses all-oses
+             :jdks ["8"]})
+          ;; shadow-cljs requires a min of jdk 11 so we'll test on that
+          [{:desc "shadow-cljs"    :cmd "bb test-shadow-cljs" :oses all-oses :jdks ["11"]
+            :skip-reason-fn (fn [{:keys [jdk]}] (when (< (parse-long jdk) 11)
+                                                  "jdk must be >= 11"))}]
+          ;; planck does not run on windows, and I don't think it needs a jdk
+          [{:desc "cljs-bootstrap" :cmd "bb test-cljs --env planck --optimizations none"
+            :oses ["macos" "ubuntu"] :jdks ["8"]}]))
 
-(defn doc-tests[]
-  (shell/command "bb test-doc"))
+(defn- ci-test-matrix []
+  (for [{:keys [desc cmd oses jdks]} (test-tasks)
+        os oses
+        jdk jdks]
+    {:desc (str desc " " os " jdk" jdk)
+     :cmd cmd
+     :os os
+     :jdk jdk}))
 
-(defn clojure-tests []
-  (doseq [version ["1.8" "1.9" "1.10" "1.11"]]
-    (shell/command "bb test-clj --clojure-version" version)) )
+(defn- local-test-list [local-os local-jdk]
+  (for [{:keys [desc cmd oses skip-reason-fn]} (test-tasks)]
+   (let [skip-reasons (cond-> []
+                        (not (some #{local-os} oses))
+                        (conj (str "os must be among " oses))
+                        (and skip-reason-fn (skip-reason-fn {:jdk local-jdk}))
+                        (conj (skip-reason-fn {:jdk local-jdk})))]
+     (cond-> {:desc desc
+              :cmd cmd}
+       (seq skip-reasons)
+       (assoc :skip-reasons skip-reasons)))))
 
-(defn cljs-tests []
-  (doseq [env ["node" "chrome-headless"]
-          opt ["none" "advanced"]]
-    (shell/command "bb" "test-cljs" "--env" env "--optimizations" opt)))
+(defn- clean []
+  (doseq [dir ["target" ".cpcache" ".shadow-cljs"]]
+    (fs/delete-file-recursively dir true)))
 
-(defn shadow-cljs-tests []
-  (shell/command "bb test-shadow-cljs"))
+(def args-usage "Valid args:
+  [matrix-for-ci [--format=json]]
+  --help
 
-(defn cljs-bootstrap-tests []
-  (if (some #{(os/get-os)} '(:mac :unix))
-    (shell/command "bb test-cljs --env planck --optimizations none")
-    (status/line :warn "skipping planck tests, they can only be run on linux and macOS")) )
+Commands:
+  matrix-for-ci Return a matrix for use within GitHub Actions workflow
+
+Options:
+  --help    Show this help
+
+By default, will run all tests applicable to your current jdk and os.")
 
 (defn -main [& args]
-  (when (main/doc-arg-opt args)
-    (clean)
-    (check-import-vars)
-    (lint)
-    (doc-tests)
-    (clojure-tests)
-    (cljs-tests)
-    (shadow-cljs-tests)
-    (cljs-bootstrap-tests))
+  (when-let [opts (main/doc-arg-opt args-usage args)]
+    (if (get opts "matrix-for-ci")
+      (let [matrix (ci-test-matrix)]
+        (if (= "json" (get opts "--format"))
+          (status/line :detail (json/generate-string matrix))
+          (do
+            (status/line :detail (doric/table [:os :jdk :desc :cmd] matrix))
+            (status/line :detail "Total jobs found: %d" (count matrix)))))
+      (let [cur-os (matrix-os)
+            cur-jdk (jdk/version)
+            cur-major-jdk (str (:major cur-jdk))
+            test-list (local-test-list cur-os cur-major-jdk)
+            tests-skipped (filter :skip-reasons test-list)
+            tests-to-run (remove :skip-reasons test-list)]
+        (when (not (some #{cur-major-jdk} all-jdks))
+          (status/line :warn "CI runs only on jdks %s but you are on jdk %s\nWe'll run tests anyway."
+                       all-jdks (:version cur-jdk)))
+
+        (status/line :head "Test plan")
+        (status/line :detail "Found %d runnable tests for jdk %s on %s:"
+                     (count tests-to-run) cur-major-jdk cur-os)
+        (doseq [{:keys [cmd]} tests-to-run]
+          (status/line :detail (str " " cmd)))
+        (doseq [{:keys [cmd skip-reasons]} tests-skipped]
+          (status/line :warn (string/join "\n* "
+                                          (concat [(str "Skipping: " cmd)]
+                                                  skip-reasons))))
+        (clean)
+        (doseq [{:keys [cmd]} tests-to-run]
+          (shell/command cmd)))))
   nil)
 
 (main/when-invoked-as-script

diff --git a/script/helper/jdk.clj b/script/helper/jdk.clj
@@ -0,0 +1,19 @@
+(ns helper.jdk
+  (:require [helper.shell :as shell]))
+
+(defn version
+  "Returns jdk version and major version with appropriate conversion. (ex 1.8 returns major of 8)"
+  []
+  (let [raw-version (->> (shell/command {:err :string}
+                                        "java -version")
+                         :err
+                         (re-find #"version \"(.*)\"")
+                         last)
+        major-minor (->> raw-version
+                         (re-find #"(\d+)(?:\.(\d+))?.*")
+                         rest
+                         (map #(when % (Integer/parseInt %))))]
+    {:version raw-version
+     :major (if (= (first major-minor) 1)
+              (second major-minor)
+              (first major-minor))}))
diff --git a/script/test_shadow_cljs.clj b/script/test_shadow_cljs.clj
@@ -1,19 +1,21 @@
 #!/usr/bin/env bb
 
 (ns test-shadow-cljs
-  (:require [helper.main :as main]
-            [helper.os :as os]
+  (:require [helper.jdk :as jdk]
+            [helper.main :as main]
             [helper.shell :as shell]
             [lread.status-line :as status]))
 
 ;; see also: shadow-cljs.edn
 (def compiled-tests "target/shadow-cljs/node-test.js")
 
 (defn -main [& args]
+  (let [{:keys [version major]} (jdk/version)]
+    (when (<= major 8)
+      (status/die 1 "shadow-cljs requires JDK 11 or above, found version %s" version)))
   (when (main/doc-arg-opt args)
     (status/line :head "testing ClojureScript source with Shadow CLJS, node, optimizations: none")
-    (shell/command (if (= :win (os/get-os)) "npx.cmd" "npx")
-                   "shadow-cljs" "compile" "test")
+    (shell/command "npx" "shadow-cljs" "compile" "test")
     (shell/command "node" compiled-tests))
   nil)