Improve documentation on test case filtering and flush error formatter on exit #133
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
OCaml's standard library only flushed the stdout formatter: https://github.com/ocaml/ocaml/blob/4.06.1/stdlib/format.ml#L1371 This meant that the 'Invalid request!' error message was never shown when an invalid filter was used. Signed-off-by: Edwin Török <edvin.torok@citrix.com>
If a filter removes everything and we are left with no tests to run then an "Invalid request!" error was shown. Point out that the problem is with the filter. Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Use Cmdliner for converting int to string, and validating the number of
arguments.
Previously just a Failure message was shown with no clear indication to
what is wrong, now more details are shown:
```diff
--- old
+++ new
@@ -1,15 +1,19 @@
+ ./simple test test_set invalidnumber
+simple: TESTCASE argument: invalid value `invalidnumber', expected an integer
+Usage: simple test [OPTION]... [NAME] [TESTCASE]
+Try `simple test --help' or `simple --help' for more information.
Testing My first test.
+ echo '$?=' 1
$?= 1
+ ./simple test nonexistent_name
+Invalid request (no tests to run, filter skipped everything)!
Testing My first test.
+ echo '$?=' 1
$?= 1
+ ./simple test test_set 1 trailingarguments
-simple: internal error, uncaught exception:
- Failure("filter_test")
-
+simple: too many arguments, don't know what to do with `trailingarguments'
+Usage: simple test [OPTION]... [NAME] [TESTCASE]
+Try `simple test --help' or `simple --help' for more information.
Testing My first test.
+ echo '$?=' 1
$?= 1
```
This does not change the semantics of what CLI arguments are accepted,
just the help text and error reporting.
From the usage example it is clear now that only 2 filters can be used.
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
Signed-off-by: Edwin Török <edvin.torok@citrix.com>
edwintorok
force-pushed
the
improve-filtering-docs
branch
from
3d93045
to
02379b2
Compare
|
Thanks! |
samoht
added a commit
to samoht/opam-repository
that referenced
this pull request
Oct 17, 2018
CHANGES: - Improve documentation for speed and tests (mirage/alcotest#129, @raphael-proust) - Improve documentation on test case filtering and flush error formatter on exit (mirage/alcotest#133, @edwintorok) - Create a fresh log sub-dir for every run (mirage/alcotest#125, @M-Harrison) - Fix wrong location hint for test files when using dune (mirage/alcotest#135, @M-Harrison)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Filtering testcases was scarcely documented (had to read the source code to find out the expected format): add an example to the README, and make --help more explicit that up to 2 arguments can be used for filtering (the name of the test, and the test case index).
Also fix a bug with error messages getting lost on exit: OCaml doesn't flush Format.stderr on exit, just Format.stdout, so we have to flush it ourselves.
The improved output compared to the old one looks like this:
No doubt the documentation could be improved further, this is just a start.