doctest
doctest copied to clipboard
Add support for asynchronous assertions through logging
The idea
Tests can use channel (value) to produce additional output besides the return value of the expression:
> ( stdout (1)
. , setTimeout (stdout, 1, 2)
. , stderr (3)
. , 4 )
[stdout]: 1
[stderr]: 3
4
[stdout]: 2
Doctest asserts that all output channels produce their output in the expected order, on the expected channel. In the above example, we even assert that [stdout]: 2 was produced asynchronously (after undefined was returned from the expression).
The philosophy is that the code example will contain exactly that which the Node REPL would when run with:
node -i -e 'const channel = x => console.log (`[channel]:`, x)'
TODO
- [x] Finish the
runfunction - [x] Add logging capabilities to CommonJS
- [x] Add logging capabilities to CoffeeScript
- [ ] Document the new feature in the README
- [ ] Try this out in Sanctuary to see if compatibility was preserved
- [ ] Try this out in Fluture to see if the solution works as intended
Feedback
- [x] https://github.com/davidchambers/doctest/pull/118#discussion_r407680275 Track output location per unique output
- [x] https://github.com/davidchambers/doctest/pull/118#issuecomment-612926078 Instead of a single logger function taking the channel name as an argument, use multiple logging functions each corresponding to a channel
- [x] https://github.com/davidchambers/doctest/pull/118#issuecomment-643594129 Figure out what to do about the breaking change
Tests
- [x] Logging in all three types of code
- [x] Synchronous logging with and without function output and exception
- [x] Asynchronous logging with and without function output and exception
- [x] Failing due to not enough output
- [x] Failing due to too much output
- [x] Failing due to incorrectly ordered output
- [x] Failing due to incorrect output channel
- [x] Failing due to timing out
- [x] Failing because the
--log-functionoption was not given
The implementation changed slightly since last time. Now I collect all "output", including log calls, into a single list during parsing. That whole list is injected as the output thunk during rewriting. The evaluator will be updated to consider not just one, but all output channels during evaluation.
The latest commit is the result of some high focus debugging. All tests but the coffeescript and commonjs are passing now. Coffeescript not passing is expected. I'm not sure why commonjs tests are not passing - they all generate the same error: require is not defined.
Hm, there might be a little more work to do before commonjs, coffeescript, and amd modules have the logging feature. I think I'll find out when writing the tests.
Tests can use
log (channel) (value)to produce additional output besides the return value of the expression:> ( log ('stdout') (1) . , setTimeout (log ('stdout'), 1, 2) . , log ('stderr') (3) ) [stdout]: 1 [stderr]: 3 undefined [stdout]: 2
Here is another interface worth considering:
> ( stdout (1)
. , setTimeout (stdout, 1, 2)
. , stderr (3) )
[stdout]: 1
[stderr]: 3
undefined
[stdout]: 2
The basic usage would then be log (f (x)) rather than log ('log') (f (x)). :)
The command-line interface would need to accept multiple log functions: --log-function stdout --log-function stderr.
The command-line interface would need to accept multiple log functions:
--log-function stdout --log-function stderr.
Oh I like that idea a lot David!
EDIT: In fact I don't want the old solution any more! :P
@davidchambers I addressed your two major points of feedback. I find myself knowing this codebase like the back of my hand, at this point. :P
Why do we restrict channel names to
[A-Za-z]+?
The restriction at this point is that a channel name must be a valid JavaScript identifier. Currently the --log-line parameter is overly permissive (any string), and the output assertion is overly restrictive (only letters). What do you think we should do?
I just rebased, squashing some non-atomic commits. For the original history, see 0688bfbe8731288d2cffc872466a92fe6a88e8e3. I also pushed each new commit separately, to ensure that the tests pass on every commit. Let's see if there's any failing commits remaining.
Despite my efforts to preserve backwards compatibility, there is at least one edge case where a user's code might break, and that's when they have code like this:
// A good old regular test
//
// > 1
// 1
// [ACommentInBracketsForSomeReason]: Why?
// That's just how I like to format my comments.
Prior to my changes, this would work well. The reason is that the state machine would only parse lines starting with // . once it reached the OUTPUT state.
In the new situation, lines starting with [chars]: are also valid once the state machine is in the OUTPUT state, because it needs to allow for asynchronous logs to occur after the expression output has been evaluated. This means that this code snippet that used to work will now generate a syntax error, as the code injector will inject Why? as the JavaScript to evaluate.
That is technically a breaking change.
Could we simply replace
[A-Za-z]+in the regular expression above with.+? -- https://github.com/davidchambers/doctest/pull/118#discussion_r438344109
Expanding the permitted characters in the brackets above will increase the odds for users to run into the issue I've described here. We could use a strict JS identifier regex, but that's a hassle.
I think the edge case would also be made much more rare if we implement your suggestion from here: https://github.com/davidchambers/doctest/pull/120#discussion_r338763923, which is in itself a breaking change.
That is technically a breaking change.
I just took a shower and regained my sanity. The now blindingly obvious solution is to parse only the words provided on the CLI. It solves the "how should our regex be" issue, and regains our backwards compat: No --log-function arg means no log output parsing.