Conditionally perform assertions, allow tests to commit to the results

[danr]

Please see #1692 (comment) for the accepted proposal and #1692 (comment) for follow-up.

---

Original issue follows:

Description: add observeTest and commitToResult

Right now we have a function test of which the essence has this type signature:

const test: (t: TestContext) => PromiseLike<void> | Iterator<any> | Observable | void

It would be helpful for library writers if there was a function:

const observeTest: 
  (test: (t: TestContext) => PromiseLike<void> | Iterator<any> | Observable | void) => 
  {success: boolean, test_result: TestResult}

edit: fixed the type signature

For my purposes TestResult can be a completely opaque type.
The use for it is to make this a result for the main driver. For the lack of a better name I will call it commitToResult for now. This could either be a function exported by AVA (or a method on AVA's exported test), or a method on TestContext:

const commitToResult: (test_result: TestResult) => void

or

interface TestContext {
    ...
    // stop the execution of the test now reporting this result:
    commitToResult: (test_result: TestResult) => void
}

Use case: property based testing

The use case for this is property-based testing, which was first introduced in Haskell with the package QuickCheck.

There are quite a few implementations for JavaScript, for example jsverify, testcheck-js. The idea is to generate random data and test if functions satsify properties run on these data to approximate universal quantification. If the test fails it is common practice in these libraries to try to shrink the generated data. Then the user can be presented with a small (and hopefully minimal) counterexample.

The library writer of a property-based testing would then run tests written using asserts as in AVA-style, but behind the hoods it would be run with observeTest, and if it fails start a loop trying to shrink the randomly generated input until it cannot be shrunk anymore and then report the smallest possible failing TestResult using commitToResult.

There is intergration for AVA in
testcheck-js/AVA-check, but it has to use the AVA internals and is thus a pain to maintain, see for example this commit in a PR by me: leebyron/testcheck-js@65ef263 So there would be clear benefits for library writers if AVA could expose these two functions.

[novemberborn]

@danr interesting!

AVA doesn't let you create new tests once tests have started running. I can't quite tell how testcheck-js gets around this. But as you say, this is about using AVA's assertion library. Perhaps we can support creating a new ExecutionContext (the t argument) that can be passed to the user's test implementation.

Taking this example:

const test = require('ava')
const { check, gen } = require('ava-check')

test('addition is commutative', check(gen.int, gen.int, (t, numA, numB) => {
  t.true(numA + numB === numB + numA)
}));

What if check() could do (in pseudocode):

function check(genA, genB, implementation) {
  return async t => {
    while (true) {
      const result = await t.try(implementation, genA(), genB())
      if (result.passed || cannotMinimizeFurther()) {
        return result.commit()
      }
    }
  }
}

With a rough type definition of:

interface ExecutionContext {
  try (implementation: (t: ExecutionContext, ...args: any[]) => void, ...args: any[]): Promise<TryResult>
}

interface TryResult {
  passed: boolean
  assertionErrors: AssertionError[]
  commit (): boolean
}

And in actual English: t.try() can be used to invoke a test implementation. It returns a promise with the result of the invocation. If passed is true then no assertions failed. Otherwise assertionErrors is populated with one or more errors. The usual rules around requiring assertions and awaiting asynchronous ones apply. commit() either passes the test or fails it with the assertionErrors. Not calling commit() results in a test failure unless another assertion is used.

I like how this is low-level, which means we can support interesting higher-order tests.

@avajs/core what do you think?

[danr]

@novemberborn: Thanks for your response and the write up.

Your try improves my observeTest but I would simplify the type to:

interface ExecutionContext {
  try (implementation: (t: ExecutionContext) => void): Promise<TryResult>
}

The call to t.try in your pseudo-code then becomes:

      const result = await t.try(t2 => implementation(t2, genA(), genB()))

Rationale: improved type-safety and smaller type signature for try.

(A name that is not a javascript keyword must be used instead of try)

Your commit improves my commitToResult becauses it is easier to use as a method.

I am a bit skeptical about this behaviour:

Not calling commit() results in a test failure unless another assertion is used.

Is this not overly restrictive? There should be no particular reason to make this a test failure by default.

[novemberborn]

Your try improves my observeTest but I would simplify the type to:

Perhaps. We have a pattern of supporting additional arguments that get passed to the implementation, seems nice to retain that.

(A name that is not a javascript keyword must be used instead of try)

It should work as a property.

I am a bit skeptical about this behaviour:

Not calling commit() results in a test failure unless another assertion is used.

Is this not overly restrictive? There should be no particular reason to make this a test failure by default.

That's how AVA behaves out of the box. Tests must use assertions. IMHO not committing a t.try is akin to not actually using assertions.

[danr]

Alright! I retract all those remarks then :)

[danr]

Any further comments on this?

Is it likely that this would be accepted upstreams?

If so can anyone give me some pointers or how to implement what @novemberborn and I have discussed?

[novemberborn]

@sindresorhus what do you think about my proposal?

[sindresorhus]

@novemberborn I like it. It's simple and can enable a lot of interesting use-cases. 👍 from me.

[novemberborn]

Expanding on my earlier comment #1692 (comment) there's the question of what to do with t.context.

We do support t.context being created in .before() hooks. We then shallowly clone it for use by each test. We're hoping this strikes a balance between letting tests (or .beforeEach() hooks) make modifications without impacting other tests.

Perhaps we should do the same when passing the execution context to the t.try() implementation? Regardless I think we can agree that assigning t.context should not affect the "parent".

Should we allow recursively calling t.try()? I suppose ideally there is no difference between t.try() implementations and test implementations. This might just be how it shakes out anyhow — so let's assume we do allow this and reconsider if it proves difficult.

Does t.try() count as one assertion for t.plan() or is it transparent? Conversely can t.try() implementations use t.plan()? I'm leaning towards it being transparent, which means that implementations mustn't be able to call t.plan(), and even when the result is committed the implementation still must have at least one passing assertion.

Ordinarily assertions are implemented in lib/assert.js but I don't think try() can do its work there. It should be added to ExecutionContext in lib/test.js instead.

Test needs to support a new assertion type, along the lines of addPendingAssertion. pendingAssertionCount should be incremented so the test fails if the promise returned by t.try() is not awaited. We need to instantiate a new ExecutionContext, but we can't pass it the Test instance. Perhaps inside the ExecutionContext we should rename "test" to "host". That way t.try() can create a new host object. There could be a Host class which is extended by Test and which implements the assertion counting and snapshot-supporting methods.

This is the point where my architecting runs into "this should really be played with in code" so I'm going to stop. @danr let me know if you want to take this on, happy to review code and give pointers!