Formatting improvements #246

craigfe · 2020-05-25T09:29:34Z

Fixes #252 and fixes #240.

No API changes, but lots of diffs to pretty-printing logic.

Shortlist of changes:

output is generally more colorful (e.g. passing around unit Fmt.t values rather than strings);
improve diff shown on Alcotest.check failure to be better aligned & spaced;
highlight the test case whose error is printed with >;
keep a more consistent distinction between ERROR and FAIL states;
show shorter directory names to the user (e.g. use the symlinks rather than UUIDs, collapse home directory to ~).

samoht · 2020-05-25T10:41:52Z

What's the difference between FAIL and ERROR?

craigfe · 2020-05-25T10:50:52Z

FAIL: test failures caused by Alcotest.fail and Alcotest.check_raises;
ERROR: test failures caused by Alcotest.check or exceptions thrown in test.

It's fairly arbitrary, but I'm preserving the existing distinction. Perhaps we want to reconsider it at this stage: for instance, it makes sense for me for Alcotest.check_raises to be considered an ERROR, or to drop the distinction entirely.

samoht · 2020-06-01T09:44:44Z

Yes we should drop the distinction. I can't remember why I thought it was a good idea initially :p

... instead of the previous state of distinguishing between `FAIL' and `ERROR' states. See the discussion in mirage#246 (comment).

craigfe · 2020-06-01T18:41:25Z

Yes we should drop the distinction.

@samoht: Now done.

New changes:

Always use FAIL rather than ERROR
Following discussion in Normalize test names to support unicode #249, display any UTF-8 characters in test-case names in unescaped form.

Also trying to sneak in some general refactoring in the internal handling of data. Mostly pushing definitions and logic into a new model.ml file that keeps some better abstraction on e.g. whether strings have been escaped, which form should be pretty-printed etc.

Happy to pull these changes into a separate PR if requested.

... instead of the previous state of distinguishing between `FAIL' and `ERROR' states. See the discussion in mirage#246 (comment).

craigfe · 2020-06-02T10:06:20Z

Rebased. This can now be considered for merge.

samoht · 2020-06-02T16:37:15Z

test/e2e/alcotest/failing/check_basic.expected

+Testing `check_basic'.
+This run has ID `<uuid>'.
+
+ ... different basic 0 bool.ASSERT bool


why is there an ASSERT boolin the middle of that line?

When using --verbose, these asserts are interleaved with the standard output.

samoht · 2020-06-02T16:38:14Z

test/e2e/alcotest/failing/check_basic.expected

+ ──────────────────────────────────────────────────────────────────────────────
+
+
+┌──────────────────────────────────────────────────────────────────────────────┐


Is the new default behaviour to show all the failures? I don't think that's a good idea, the previous default is just to show the first one unless ALCOTEST_SHOW_ERRORS is set (if I remember correctly).

No, this isn't default. There's a corresponding change to the Alcotest.run to pass ~verbose:true (https://github.com/mirage/alcotest/pull/246/files#diff-fb43ad350d97f3cf0e25d21ec53a881dR7).

I felt that this is a better test if all of the errors are printed.

src/alcotest-engine/alcotest_engine.mli

The 'latest' symlink is less useful in projects with many different test binaries (such as this one), since each suite runner overwrites the symlink from the previous one.

This test causes plenty of trouble in CI, and seems to be not very portable. The important test is that it's possible to _override_ the default, which has been kept.

... instead of the previous state of distinguishing between `FAIL' and `ERROR' states. See the discussion in mirage#246 (comment).

craigfe · 2020-06-05T13:36:49Z

Rebased, and switched out the last change for one that doesn't use brands.

... by switching to an escaped form when using the suite name as a symlink. The pretty-printed form is unaffected.

craigfe · 2020-06-16T15:34:47Z

Pushed a small addition to the UTF-8 improvement that also handles printing of UTF-8 characters in suite names (previously we were just dealing with test names). This now fixes #240.

Now is a good time for a second review 🙂

samoht · 2020-07-02T13:19:16Z

This looks very good, thanks!

@hannesm

…age (1.2.0) CHANGES: - Add an `alcotest-mirage` package, allowing the construction of MirageOS unikernels that run Alcotest test suites. (mirage/alcotest#238, @hannesm @linse) - Add `Alcotest.check'`, a variant of `Alcotest.check` with labeled arguments. (mirage/alcotest#239, @hartmut27) - Add a testable for the `bytes` type. (mirage/alcotest#253, @mefyl) - Many assorted improvements to Alcotest output formatting. (mirage/alcotest#246, @craigfe) - Default to `--color=always` when running inside Dune (mirage/alcotest#242, @craigfe). The value can be overridden by setting the `ALCOTEST_COLOR` variable in a `dune` file, for example: ```dune (env (_ (env-vars (ALCOTEST_COLOR auto)))) ``` - Support all UTF-8 characters in test names and suite names, by normalising them for file-system interactions. (mirage/alcotest#249, @gs0510; mirage/alcotest#246, @craigfe) - Fix various crashes when using non-filesystem-safe characters in test suite names (these break Alcotest when attempting to generate a corresponding log file). (mirage/alcotest#241, @mefyl; mirage/alcotest#246 @craigfe)

craigfe mentioned this pull request May 25, 2020

Subcomponent diffing #247

Closed

7 tasks

craigfe force-pushed the layout-improvements branch 2 times, most recently from e9997e3 to 02080c2 Compare May 25, 2020 11:30

This was referenced May 26, 2020

Normalize test names to support unicode #249

Merged

Expected and actual output do not stand enough in the output #252

Closed

craigfe force-pushed the layout-improvements branch from 02080c2 to 5c32f40 Compare June 1, 2020 11:15

craigfe added a commit to craigfe/alcotest that referenced this pull request Jun 1, 2020

Test failures are always marked with `FAIL'

86e766f

... instead of the previous state of distinguishing between `FAIL' and `ERROR' states. See the discussion in mirage#246 (comment).

craigfe force-pushed the layout-improvements branch 6 times, most recently from 4e3eaf2 to 2b9e01c Compare June 1, 2020 18:40

craigfe added a commit to craigfe/alcotest that referenced this pull request Jun 2, 2020

Test failures are always marked with `FAIL'

300a200

... instead of the previous state of distinguishing between `FAIL' and `ERROR' states. See the discussion in mirage#246 (comment).

craigfe force-pushed the layout-improvements branch from 2b9e01c to 41b72fa Compare June 2, 2020 10:03

samoht reviewed Jun 2, 2020

View reviewed changes

src/alcotest-engine/alcotest_engine.mli Outdated Show resolved Hide resolved

craigfe added 7 commits June 5, 2020 15:02

Indent printed events slightly

9e5af13

Collapse $HOME' path prefixes to ~' in printed output

e3d7970

Standardise printing of ERROR and FAIL messages

8f12eae

Fix some degenerate uses of Format strings

8013d5f

Ensure summary is printed inside a vbox

5da72c8

Use coloured --compact output

f2e8dbd

Wrap --compact output to the terminal width

ed39d2f

craigfe added 12 commits June 5, 2020 15:02

Add space between Expected' and Received' when wrapping occurs

4a9b776

Use FAIL rather than ERROR in results when appropriate

72e2824

Suggest exe_name' symlink rather than latest' in output

9cb1d22

The 'latest' symlink is less useful in projects with many different test binaries (such as this one), since each suite runner overwrites the symlink from the previous one.

Left-align exception backtraces in results

df3124e

Refactor log_dir symlink printing

c55850d

Shift horizontal rule formatter to Pp

ff2d485

Preserve colours in Alcotest.fail output

2deb8d3

Avoid use of Fun for pre-4.08 compatibility

29dcfd0

Re-implement List.init for pre-4.06 compatibility

f641ac7

Scrap the test of default colouring in Dune

0c6a609

This test causes plenty of trouble in CI, and seems to be not very portable. The important test is that it's possible to _override_ the default, which has been kept.

Don't reference created symlinks on Windows

4a9d70b

Test failures are always marked with `FAIL'

a47603e

... instead of the previous state of distinguishing between `FAIL' and `ERROR' states. See the discussion in mirage#246 (comment).

craigfe force-pushed the layout-improvements branch from 41b72fa to f321ced Compare June 5, 2020 13:36

Display UTF-8 characters in terminal output

e6d77bc

craigfe force-pushed the layout-improvements branch from f321ced to e6d77bc Compare June 5, 2020 14:17

Handle non-filesystem-safe characters in test suite names

b8eab03

... by switching to an escaped form when using the suite name as a symlink. The pretty-printed form is unaffected.

samoht merged commit 1c51b8f into mirage:master Jul 2, 2020

craigfe mentioned this pull request Jul 13, 2020

[new release] alcotest-lwt, alcotest, alcotest-async and alcotest-mirage (1.2.0) ocaml/opam-repository#16806

Merged

craigfe deleted the layout-improvements branch August 18, 2020 14:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Formatting improvements #246

Formatting improvements #246

craigfe commented May 25, 2020 •

edited

Loading

samoht commented May 25, 2020

craigfe commented May 25, 2020 •

edited

Loading

samoht commented Jun 1, 2020

craigfe commented Jun 1, 2020

craigfe commented Jun 2, 2020

samoht Jun 2, 2020

craigfe Jun 2, 2020

samoht Jun 2, 2020

craigfe Jun 2, 2020

craigfe commented Jun 5, 2020

craigfe commented Jun 16, 2020

samoht commented Jul 2, 2020 •

edited

Loading

		──────────────────────────────────────────────────────────────────────────────


		┌──────────────────────────────────────────────────────────────────────────────┐

Formatting improvements #246

Formatting improvements #246

Conversation

craigfe commented May 25, 2020 • edited Loading

samoht commented May 25, 2020

craigfe commented May 25, 2020 • edited Loading

samoht commented Jun 1, 2020

craigfe commented Jun 1, 2020

craigfe commented Jun 2, 2020

samoht Jun 2, 2020

Choose a reason for hiding this comment

craigfe Jun 2, 2020

Choose a reason for hiding this comment

samoht Jun 2, 2020

Choose a reason for hiding this comment

craigfe Jun 2, 2020

Choose a reason for hiding this comment

craigfe commented Jun 5, 2020

craigfe commented Jun 16, 2020

samoht commented Jul 2, 2020 • edited Loading

craigfe commented May 25, 2020 •

edited

Loading

craigfe commented May 25, 2020 •

edited

Loading

samoht commented Jul 2, 2020 •

edited

Loading