Skip to content

Commit

Permalink
test: include full coverage for windows (#122)
Browse files Browse the repository at this point in the history
Instead of launching os specific commands we now launch the os-agnostic
wee dummy `test/wd.clj` script via babashka. It is designed to satisfy
the needs of tests.

Examples of types of changes to launched processes:
- echo hello -> bb script/wd.clj :out hello
- ls -> bb script/wd.clj :ls .
- grep somestring -> bb script/wd.clj :grep somestring
- clojure -e some-expr -> bb script/wd.clj :out boo :err bah :exit 1

Replaced most uses of `cat` with `bb script/wd.clj :upper`, I felt
that transforming the input offered more proof that the process was run
successfully.

Because our wee dummy script allows us to define explicit output and
behaviour, tests assertions are now also often more explicit.

Took care to keep the args syntax the same as the original test.
(i.e. args as string vs vector of strings vs vector of symbols).

CI downloads babashka deps prior to test run. This avoids deps download
messages to stderr interfering with the test. A `bb --version` is sufficient.

Removed Windows specific tests, tests are now OS agnostic:
- windows-invoke-git-with-space-test -> process-space-in-cmd-test
- windows-executable-resolver-test -> tested throughout
- windows-pprint-test -> pprint-test
- windows-pre-start-fn-test -> pre-start-fn-test

Understood that tests can be launched from babashka or babashka/process.
When tests are run from babashka for the jvm, and bb does not exist, we
skip and pass tests that require bb with a warning. These same tests
will be run with natively with bb in a subsequent pass, so we are not
reducing babashka test coverage.

Updated process-dir-option-test:
- No longer skipping a test assertion when run under babashka.
- Deleted a redundant assertion.

Observations when requesting an empty :env noted in README:
- Windows always includes `SystemRoot`
- macOS always includes `__CF_USER_TEXT_ENCODING`

GitHub Actions changes (could be considered out of scope):
- I set fail-fast to false for the matrix, I found it helpful to all
  jobs run even after one job had failed.
- I adjusted deps caching to include ~/.deps.clj, I found this helpful
  in validating my explorations in what deps I needed to pre-download.
- Noticed bb.edn was missing from deps cache key so added it in.

Made some other very minor changes to code/comments for clarity.

Closes #116
  • Loading branch information
lread authored May 8, 2023
1 parent 84919a6 commit 748b098
Show file tree
Hide file tree
Showing 6 changed files with 422 additions and 295 deletions.
2 changes: 2 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ jobs:
name: Install Babashka
command: |
sudo bash < <(curl -s https://raw.githubusercontent.com/babashka/babashka/master/install)
bb --version
- run:
name: Run JVM tests
command: |
Expand Down Expand Up @@ -58,6 +59,7 @@ jobs:
name: Install Babashka
command: |
sudo bash < <(curl -s https://raw.githubusercontent.com/babashka/babashka/master/install)
bb --version
- run:
name: Run JVM tests
command: |
Expand Down
29 changes: 13 additions & 16 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ on: [push, pull_request]
jobs:
test:
strategy:
fail-fast: false
matrix:
java-version: ["8", "11", "17"]
os: [ubuntu-latest, macOS-latest, windows-latest]
Expand All @@ -24,28 +25,24 @@ jobs:
- name: "Restore Cache"
uses: "actions/cache@v3"
with:
path: "~/.m2/repository"
key: "${{ runner.os }}-deps-${{ hashFiles('deps.edn') }}"
restore-keys: "${{ runner.os }}-deps-"
path: |
~/.m2/repository
~/.gitlibs
~/.deps.clj
key: "${{ runner.os }}-deps-${{ hashFiles('deps.edn','bb.edn') }}"

- name: Setup Clojure
uses: DeLaGuardo/[email protected]
with:
cli: 1.10.3.1040
bb: 'latest'

- name: Run tests not Windows
if: ${{ matrix.os != 'windows-latest' }}
run: |
clojure -M:clj-1.9:test -e windows
clojure -M:clj-1.10:test -e windows
clojure -M:clj-1.11:test -e windows
shell: bash
- name: Download bb deps
run:
bb --version

- name: Run tests on Windows
if: ${{ matrix.os == 'windows-latest' }}
- name: Run tests
run: |
clojure -M:clj-1.9:test -i windows
clojure -M:clj-1.10:test -i windows
clojure -M:clj-1.11:test -i windows
shell: powershell
clojure -M:clj-1.9:test
clojure -M:clj-1.10:test
clojure -M:clj-1.11:test
3 changes: 3 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,9 @@ The `:env` option replaces your entire environment with the provided map. To add
For example, `"PATH"` will not update the value of `"Path"` on Windows.
Here's an [example of a babashka task](https://github.com/babashka/fs/blob/3b8010d1a0db166771ac7f47573ea09ed45abe33/bb.edn#L10-L11) that understands this nuance.

> **:env TIP**: An OS might have default environment variables it always includes.
For example, as of this writing, Windows always includes `SystemRoot` and macOS always includes `__CF_USER_TEXT_ENCODING`.

## Pipelines

The `pipeline` function returns a
Expand Down
2 changes: 1 addition & 1 deletion script/test
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
#!/usr/bin/env bash

clojure -M:test -e windows @$
clojure -M:test @$
27 changes: 27 additions & 0 deletions script/wd.clj
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
;; wd - wee dummy - an os-agnostic bb script launched by our unit tests
(require '[clojure.java.io :as io]
'[clojure.string :as str])

;; usage
;; :out <somestring> - dump <somestring> to stdout
;; :err <somestring> - dump <somestring> to stderr
;; :ls <somefile> - dir of <somefile> to stdout
;; :env - dump env to stdout
;; :grep <somestring> - returns all lines from stdin matching <somestring>
;; :upper - read and emit lines from stdin, but converted to uppercase
;; :exit <someval> - exits with <someval>

;; the naivest of cmd line parsing
(doseq [[cmd val] (partition-all 2 1 *command-line-args*)]
(case cmd
":out" (println val)
":err" (binding [*out* *err*] (println val))
":ls" (pr (->> val io/file (.listFiles) (map str) sort))
":env" (pr (->> (System/getenv) (into {})))
":grep" (doseq [l (->> *in* io/reader line-seq (filter #(str/includes? % val)))]
(println l))
":upper" (doseq [l (->> *in* io/reader line-seq)]
(println (str/upper-case l)))
":sleep" (Thread/sleep (parse-long val))
":exit" (System/exit (parse-long val))
nil))
Loading

0 comments on commit 748b098

Please sign in to comment.