Improve reporting for test scripts run by patchers

Description

Expected Behavior

Any failure from ./script/test results results in a non-zero exit code and some kind of indication of the failure in the summary.
This does not mean we fail fast, we run all available target environments as we do currently.

Actual Behavior

The summary does report the number of environments tested (e.g. Tested with 6 out of 6 possible js targets) but gives no indication in summary of failures in those environments.
Also always exits with 0.
Because ./script/test produces a good amount of output and failures are buried in that output, it is too easy to assume success when something has failed.

- this work should be compatible with Git Bash on Windows

See also

./script/test-simple

Analysis

Cursory tests show that ./script/test-simple is not running, will need to add :main to compiler options.

Approach

  1. Move duplicated code from ./script/test and ./script/test-simple to shared ./script/test-runner.

  2. Produce 1 line of summary for each test target conveying status of pass, fail or skip.

  3. Exit with 0 (success) when no failures and at least 1 target passed (consider target skips, if not all skipped, ok), otherwise exit with 1 (failure).

  4. Colorize key outputs for at-a-glance parsing.

  5. honor NO_COLOR (https://no-color.org/) for folks that do not like colorized output

Screenshots

To help the reviewer, I have included some screenshots of the new summary section.

Success

The following will return exit status 0

With NO_COLOR:

Failures

The following will return exit status 1

Implementation notes

  1. Switched from !#/bin/sh to !#/bin/env bash to make life easier. I don't think this is controversial, but thought it worth raising.

  2. Windows script/test.ps1 was not considered as part of this work.

  3. Applied CLSJ-3098 fix for GraalVM on Windows.

Observations

  1. Continuous Integration does not run ./script/test. As a patcher, it would be good to know that I am executing the same tests as CI, but this would be the subject of a separate ticket.

Testing notes

Scripted execution then manual verification of results on

  1. macOS Mojave

  2. linux via local Docker

  3. Windows 10 via ssh to local Windows VM

Scripted tests and resulting logs are attached in cljs-3075...zip file. These scripts are specific to my setup but do include Dockerfile used for linux. Entry-point is verify.sh.

Of note:

  1. bin/cljsc and bin/classpath_conv were applied to patched versions in testing to support Windows runs. A side effect was that this also resolved test failures on my particular installation of linux due to file.encoding not being utf-8.

  2. script/test did not work on Windows prior to this patch and it will still not work on Windows until CLSJ-3098 is applied.

Environment

None

Activity

Show:
Mike Fikes
June 26, 2019, 5:48 AM

CLJS-3075-2.patch added to Patch Tender

Lee Read
June 22, 2019, 6:03 PM

Ok, this one is ready for review. I thought visualizations were lacking, so I added some to the description to hopefully make review easier. Thanks, and let me know what you think.

Lee Read
June 22, 2019, 4:33 PM

Added CLSJ-3075-2.patch which:

  • includes fix for GraalVM on Windows

  • gets rid of usage of controversial bash eval

  • fixes an issue for grepping for failures. See CLSJ-3114

Lee Read
June 16, 2019, 9:16 PM

I will come back to this one. It needs a tweak to be compatible with Git Bash on Windows.

Mike Fikes
May 26, 2019, 1:01 PM

CLJS-3075.patch added to Patch Tender

Assignee

Mike Fikes

Reporter

Lee Read

Labels

None

Approval

None

Patch

Code

Priority

Minor