Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for asynchronous assertions through logging #118

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Avaq
Copy link
Collaborator

@Avaq Avaq commented May 21, 2019

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

  • Finish the run function
  • Add logging capabilities to CommonJS
  • 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

Tests

  • Logging in all three types of code
  • Synchronous logging with and without function output and exception
  • Asynchronous logging with and without function output and exception
  • Failing due to not enough output
  • Failing due to too much output
  • Failing due to incorrectly ordered output
  • Failing due to incorrect output channel
  • Failing due to timing out
  • Failing because the --log-function option was not given

@Avaq

This comment has been minimized.

@Avaq

This comment has been minimized.

lib/doctest.js Outdated Show resolved Hide resolved
@Avaq Avaq self-assigned this Apr 12, 2020
@Avaq Avaq marked this pull request as ready for review April 12, 2020 15:21
@Avaq Avaq requested a review from davidchambers April 12, 2020 15:21
@Avaq

This comment has been minimized.

.travis.yml Outdated Show resolved Hide resolved
lib/common.js Outdated Show resolved Hide resolved
lib/common.js Outdated Show resolved Hide resolved
lib/common.js Outdated Show resolved Hide resolved
lib/doctest.js Outdated Show resolved Hide resolved
lib/doctest.js Outdated Show resolved Hide resolved
lib/doctest.js Outdated Show resolved Hide resolved
lib/doctest.mjs Outdated Show resolved Hide resolved
lib/doctest.mjs Outdated
Comment on lines 52 to 54
'const ' + logFunction + ' = channel => value => {',
' __doctest.logMediator.emit ({channel, value});',
'};',
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is our first use of arrow functions. Should we also use () => {} in place of function() {} a few lines above?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh. Perhaps I should use a regular function.

lib/doctest.mjs Outdated Show resolved Hide resolved
test/index.js Outdated Show resolved Hide resolved
lib/common.js Outdated Show resolved Hide resolved
lib/doctest.js Outdated Show resolved Hide resolved
lib/doctest.js Outdated Show resolved Hide resolved
lib/doctest.js Outdated Show resolved Hide resolved
@davidchambers

This comment has been minimized.

@Avaq

This comment has been minimized.

@Avaq

This comment has been minimized.

@Avaq

This comment has been minimized.

lib/program.js Outdated Show resolved Hide resolved
lib/program.js Outdated Show resolved Hide resolved
lib/doctest.mjs Outdated Show resolved Hide resolved
lib/doctest.js Outdated Show resolved Hide resolved
@Avaq

This comment has been minimized.

@Avaq

This comment has been minimized.

value: value
});
output ($test);
if (!$test[OUTPUT].some (isStandardOutput)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This if statement actually describes another state in our state machine implicitly. It would not have to be there if we had two separate states for logging: SYNC_LOG and ASYNC_LOG. While the state machine is in SYNC_LOG, it allows transitions to the OUTPUT state, but while it's in ASYNC_LOG it only allows transitions to itself or CLOSED.

It might be nicer to implement it like that.

Copy link
Collaborator Author

@Avaq Avaq Jun 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The valid state transitions would then be something like:

CLOSED -> CLOSED
CLOSED -> OPEN

OPEN -> INPUT

INPUT -> INPUT
INPUT -> MORE_INPUT
INPUT -> SYNC_LOG
INPUT -> OUTPUT
INPUT -> ASYNC_LOG
INPUT -> CLOSED

MORE_INPUT -> MORE_INPUT
MORE_INPUT -> SYNC_LOG
MORE_INPUT -> OUTPUT
MORE_INPUT -> ASYNC_LOG
MORE_INPUT -> CLOSED

SYNC_LOG -> SYNC_LOG
SYNC_LOG -> MORE_SYNC_LOG
SYNC_LOG -> OUTPUT
SYNC_LOG -> ASYNC_LOG
SYNC_LOG -> INPUT
SYNC_LOG -> CLOSED

MORE_SYNC_LOG -> SYNC_LOG
MORE_SYNC_LOG -> MORE_SYNC_LOG
MORE_SYNC_LOG -> OUTPUT
MORE_SYNC_LOG -> ASYNC_LOG
MORE_SYNC_LOG -> INPUT
MORE_SYNC_LOG -> CLOSED

OUTPUT -> MORE_OUTPUT
OUTPUT -> ASYNC_LOG
OUTPUT -> INPUT
OUTPUT -> CLOSED

MORE_OUTPUT -> MORE_OUTPUT
MORE_OUTPUT -> ASYNC_LOG
MORE_OUTPUT -> INPUT
MORE_OUTPUT -> CLOSED

ASYNC_LOG -> ASYNC_LOG
ASYNC_LOG -> MORE_ASYNC_LOG
ASYNC_LOG -> INPUT
ASYNC_LOG -> CLOSED

MORE_ASYNC_LOG -> ASYNC_LOG
MORE_ASYNC_LOG -> MORE_ASYNC_LOG
MORE_ASYNC_LOG -> INPUT
MORE_ASYNC_LOG -> CLOSED

@Avaq
Copy link
Collaborator Author

Avaq commented Jun 13, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants