Improved test partitioning: keep namespaces grouped together and don'…

…t sort tests (#31)

* Improved test partitioning

* Update dox

* Code cleanup

* Modernize GH Actions

* Appease Kondo

.github/actions/setup/action.yml

@@ -0,0 +1,34 @@
+name: Setup Clojure
+inputs:
+  clojure-version:
+    required: true
+    default: "1.11.1.1413"
+  java-version:
+    required: true
+    default: "17"
+  cache-key:
+    required: true
+
+runs:
+  using: composite
+  steps:
+    - name: Prepare JDK
+      uses: actions/setup-java@v3
+      with:
+        java-version: ${{ inputs.java-version }}
+        distribution: 'temurin'
+    - name: Setup Clojure
+      uses: DeLaGuardo/[email protected]
+      with:
+        cli: ${{ inputs.clojure-version }}
+    - name: Restore cache
+      uses: actions/cache@v3
+      with:
+        path: |
+          ~/.m2/repository
+          ~/.gitlibs
+          ~/.deps.clj
+        key: v1-${{ hashFiles('./deps.edn') }}-${{ inputs.cache-key }}
+        restore-keys: |
+          v1-${{ hashFiles('./deps.edn') }}-
+          v1-

.github/workflows/deploy.yml

@@ -10,35 +10,18 @@ jobs:
     runs-on: ubuntu-20.04
     environment: Deployment
     steps:
-    - uses: actions/[email protected]
-      with:
-        fetch-depth: 0
-    - name: Prepare JDK 17
-      uses: actions/setup-java@v3
-      with:
-        java-version: 17
-        distribution: 'temurin'
-    - name: Setup Clojure
-      uses: DeLaGuardo/[email protected]
-      with:
-        cli: 1.11.1.1413
-    - name: Restore cache
-      uses: actions/cache@v3
-      with:
-        path: |
-          ~/.m2/repository
-          ~/.gitlibs
-          ~/.deps.clj
-        key: v1-${{ hashFiles('./deps.edn') }}-deploy
-        restore-keys: |
-          v1-${{ hashFiles('./deps.edn') }}-
-          v1-
-    - name: Build Hawk
-      run: clojure -T:build jar
-      env:
-        GITHUB_SHA: ${{ env.GITHUB_SHA }}
-    - name: Deploy Hawk
-      run: clojure -T:build deploy
-      env:
-        CLOJARS_USERNAME: ${{ secrets.CLOJARS_USERNAME }}
-        CLOJARS_PASSWORD: ${{ secrets.CLOJARS_PASSWORD }}
+      - uses: actions/[email protected]
+      - uses: ./.github/actions/setup
+        with:
+          cache-key: deploy
+      - name: Build Hawk
+        run: >-
+          clojure -T:build jar
+        env:
+          GITHUB_SHA: ${{ env.GITHUB_SHA }}
+      - name: Deploy Hawk
+        run: >-
+          clojure -T:build deploy
+        env:
+          CLOJARS_USERNAME: ${{ secrets.CLOJARS_USERNAME }}
+          CLOJARS_PASSWORD: ${{ secrets.CLOJARS_PASSWORD }}

.github/workflows/tests.yml

@@ -2,62 +2,33 @@ name: Tests
 
 on:
   push:
-  workflow_dispatch:
-    inputs:
-      debug_enabled:
-        type: boolean
-        description: 'Run the build with tmate debugging enabled (https://github.com/marketplace/actions/debugging-with-tmate)'
-        required: false
-        default: false
+    branches:
+      - main
+  pull_request:
 
 jobs:
   kondo:
     runs-on: ubuntu-20.04
     timeout-minutes: 10
     steps:
-    - uses: actions/checkout@v3
-    - uses: DeLaGuardo/clojure-lint-action@master
-      with:
-        check-name: Run clj-kondo
-        clj-kondo-args: >-
-          --lint
-          src
-          test
-        github_token: ${{ secrets.GITHUB_TOKEN }}
+      - uses: actions/[email protected]
+      - uses: ./.github/actions/setup
+        with:
+          cache-key: kondo
+      - name: Run Kondo
+        run: >-
+          clojure -M:kondo --lint src test
 
   tests:
     runs-on: ubuntu-20.04
     timeout-minutes: 10
     steps:
-    - uses: actions/checkout@v3
-    - name: Prepare JDK 17
-      uses: actions/setup-java@v3
-      with:
-        java-version: 17
-        distribution: 'temurin'
-    - name: Setup Clojure
-      uses: DeLaGuardo/[email protected]
-      with:
-        cli: 1.11.1.1208
-    - name: Restore cache
-      uses: actions/cache@v3
-      with:
-        path: |
-          ~/.m2/repository
-          ~/.gitlibs
-          ~/.deps.clj
-        key: v1-${{ hashFiles('./deps.edn') }}-postgres
-        restore-keys: |
-          v1-${{ hashFiles('./deps.edn') }}-
-          v1-
-    # to continue the workflow create an empty file `touch continue`
-    # or `sudo touch /continue` while still in an SSH session
-    - name: Setup tmate session
-      if: ${{ github.event_name == 'workflow_dispatch' && inputs.debug_enabled }}
-      uses: mxschmitt/action-tmate@v3
-      with:
-        limit-access-to-actor: true
-    - run: clojure -X:dev:test
-      name: Run tests
-      env:
-        CI: TRUE
+      - uses: actions/[email protected]
+      - uses: ./.github/actions/setup
+        with:
+          cache-key: tests
+      - name: Run tests
+        run: >-
+          clojure -X:dev:test
+        env:
+          CI: TRUE

README.md

@@ -218,12 +218,13 @@ Running 575 tests
 ...
 ```
 
-`:partition/index` is zero-based, e.g. if you have ten partitions (`:partiton/total 10`) then the first partition is `0` and
-the last is `9`.
+`:partition/index` is zero-based, e.g. if you have ten partitions (`:partiton/total 10`) then the first partition is `0`
+and the last is `9`.
 
-Tests are partitioned at the `deftest` level after all tests are found the usual way -- all namespaces that would be
-loaded if you were running the entire test suite are still loaded. Partitions are split as evenly as possible, but
-tests are guaranteed to be split deterministically into exactly the number of partitions you asked for.
+Tests are partitioned at the var (`deftest`) level after all tests are found the usual way, but all tests in any given
+namespace will always be split into the same partition. All namespaces that would be loaded if you were running the
+entire test suite are still loaded. Partitions are split as evenly as possible, but tests are guaranteed to be split
+deterministically into exactly the number of partitions you asked for.
 
 
 ## Additional options

deps.edn

@@ -28,5 +28,20 @@
                 slipset/deps-deploy           {:mvn/version "0.2.1"}}
    :ns-default build}
 
+  ;; clojure -M:kondo --lint src test
+  ;;
+  ;; clojure -M:kondo --version
+  ;;
+  ;; clojure -M:kondo --copy-configs --dependencies --lint "$(clojure -A:dev -Spath)" --skip-lint --parallel
+  ;;
+  ;; Run Kondo from the JVM using the pinned version. Preferable to running the installed command since we can pin the
+  ;; version here which may be different from the version installed on your computer.
+  :kondo
+  {:replace-deps
+   {clj-kondo/clj-kondo {:mvn/version "2024.08.29"}}
+
+   :main-opts
+   ["-m" "clj-kondo.main"]}
+
   :ci
   {:jvm-opts ["-Dhawk.mode" "cli/ci"]}}}

src/mb/hawk/core.clj

@@ -2,7 +2,6 @@
   (:require
    [clojure.java.classpath :as classpath]
    [clojure.java.io :as io]
-   [clojure.math :as math]
    [clojure.pprint :as pprint]
    [clojure.set :as set]
    [clojure.string :as str]
@@ -17,6 +16,7 @@
    [mb.hawk.init :as hawk.init]
    [mb.hawk.junit :as hawk.junit]
    [mb.hawk.parallel :as hawk.parallel]
+   [mb.hawk.partition :as hawk.partition]
    [mb.hawk.speak :as hawk.speak]
    [mb.hawk.util :as u]))
 
@@ -121,47 +121,6 @@
   [_nil options]
   (find-tests (classpath/system-classpath) options))
 
-(defn- partition-all-into-n-partitions
-  "Split sequence `xs` into `num-partitions` as equally as possible. Guaranteed to return `num-partitions`. This custom
-  function is used instead of [[partition-all]] or whatever because we want to make sure every partition gets tests,
-  even with weird combinations like 4 tests with 3 partitions or 29 tests with 10 partitions."
-  [num-partitions xs]
-  {:post [(= (count %) num-partitions)]}
-  ;; make sure the partitioning is deterministic -- `xs` should always come back in the same order but we should sort
-  ;; just to be safe.
-  (let [xs             (sort-by str xs)
-        partition-size (/ (count xs) num-partitions)]
-    (into []
-          (comp (map-indexed (fn [i x]
-                               [(long (math/floor (/ i partition-size))) x]))
-                (partition-by first)
-                (map (fn [partition]
-                       (map second partition))))
-          xs)))
-
-(defn- partition-tests [tests {num-partitions :partition/total, partition-index :partition/index, :as _options}]
-  (if (or num-partitions partition-index)
-    (do
-      (assert (and num-partitions partition-index)
-              ":partition/total and :partition/index must be set together")
-      (assert (pos-int? num-partitions)
-              "Invalid :partition/total - must be a positive integer")
-      (assert (<= num-partitions (count tests))
-              "Invalid :partition/total - cannot have more partitions than number of tests")
-      (assert (int? partition-index)
-              "Invalid :partition/index - must be an integer")
-      (assert (<= 0 partition-index (dec num-partitions))
-              (format "Invalid :partition/index - must be between 0 and %d" (dec num-partitions)))
-      (let [partitions (partition-all-into-n-partitions num-partitions tests)
-            partition  (nth partitions partition-index)]
-        (printf "Running tests in partition %d of %d (%d tests of %d)...\n"
-                (inc partition-index)
-                num-partitions
-                (count partition)
-                (count tests))
-        partition))
-    tests))
-
 (defn find-tests-with-options
   "Find tests using the options map as passed to `clojure -X`."
   [{:keys [only], :as options}]
@@ -170,7 +129,7 @@
     (println "Running tests in" (pr-str only)))
   (let [start-time-ms (System/currentTimeMillis)
         tests         (-> (find-tests only options)
-                          (partition-tests options))]
+                          (hawk.partition/partition-tests options))]
     (printf "Finding tests took %s.\n" (u/format-milliseconds (- (System/currentTimeMillis) start-time-ms)))
     (println "Running" (count tests) "tests")
     tests))