Skip to content

Feature Request: A way to mark a test 'skipped' after it starts #2010

Open
@coreyfarrell

Description

@coreyfarrell

Description

I'm using ava to control selenium-webdriver for tests run in Firefox and Chrome. It cannot be known synchronously if the browser can be run. An example flow could be:

test('somepage.html', async t => {
  try {
    await loadPage('somepage.html');
  } catch (error) {
    t.skip();
    return;
  }
  // testing against the page here
});

In this example loadPage() will reject if the browser cannot be started, otherwise it will resolve. The goal is that the test will report skipped if the browser cannot be run.

It would probably be good if t.skip() threw an exception if any assertions have already run for that test.

Environment

Node.js v10.14.2
linux 4.19.8-200.fc28.x86_64
ava 1.0.1
npm 6.4.1

Activity

novemberborn

novemberborn commented on Jan 11, 2019

@novemberborn
Member

That's an interesting use case. Calling it skip may be misleading, since the test did execute. We have t.pass() and t.fail(). Perhaps t.warn()?

Combined with #1692 you could even let some assertions fail, discard them, and then warn that the test could not be completed.

@sindresorhus thoughts?

coreyfarrell

coreyfarrell commented on Jan 11, 2019

@coreyfarrell
ContributorAuthor

My hope is for the final report to report to state that tests for unavailable browsers to be 'skipped'. Specifically when a specific browser cannot be run it should not report pass or fail for the associated tests.

BTW I've edited my example code to show that you would return from the test after calling t.skip(). I'm open to different naming, maybe t.skipped() or t.dependencyMissing()?

novemberborn

novemberborn commented on Jan 11, 2019

@novemberborn
Member

Yes that was my understanding. I'm proposing that t.warn() puts the test in a state where it has neither failed nor passed. It wouldn't impact the exit code. You could still return, perhaps we should make that mandatory — e.g. if there is another failing assertion in the same test that would override the warning.

coreyfarrell

coreyfarrell commented on Jan 11, 2019

@coreyfarrell
ContributorAuthor

Agreed that it should be mandatory to return after calling this function. One hesitation about t.warn is that you have t.log. I've always felt that t.warn should exist and be similar to console.warn the same way t.log is like console.log. Not sure if that could cause confusion?

novemberborn

novemberborn commented on Jan 11, 2019

@novemberborn
Member

Yes I was wondering about warn… if we can do some bike shedding, the problem is that the test can't pass due to circumstances out of its control, and yet this shouldn't fail CI. Perhaps t.cancel()?


That said, is it feasible to determine whether the test can run before you declare any tests? Once you call test() (or any hooks) you must declare all tests synchronously, but that first call is allowed to be asynchronous.

coreyfarrell

coreyfarrell commented on Jan 11, 2019

@coreyfarrell
ContributorAuthor

t.cancel definitely seems better than t.warn.


Very interesting, I didn't know it was possible to defer the first registration to ava. This might be a solution to my specific problem, though honestly something like t.cancel would be easier.

coreyfarrell

coreyfarrell commented on Feb 17, 2019

@coreyfarrell
ContributorAuthor

I've tweaked the way my tests are declared so they do not get created until after the browser is successfully started, though this does have one drawback. An example test declaration is:

page('check-text.html', async t => {
	const {selenium, checkText} = t.context;
	const ele = await selenium.findElement({id: 'test'});

	await checkText(ele, 'This is a test!');
});

The page function adds the title and callback to an internal array, then after the browser is successfully started it performs a bunch of calls to test() or test.skip(). If this results in an exception due to duplicate test name then the backtrace is wrong, so having page directly call test() with an implementation that conditionally calls t.cancel() would be preferable for the purpose of duplicate title error reporting. This is a minor issue though.

changed the title [-]Feature Request: A way to mark a test 'skipped' after it starts.[/-] [+]Feature Request: A way to mark a test 'skipped' after it starts[/+] on Feb 18, 2019
sindresorhus

sindresorhus commented on Feb 18, 2019

@sindresorhus
Member

I like the idea (and naming) of t.cancel(). I can see that being useful for many situations.

coreyfarrell

coreyfarrell commented on Feb 18, 2019

@coreyfarrell
ContributorAuthor

So in my use case t.cancel() would be called before any t assertions and be followed by an immediate return:

test('test with async resolvable external dependency', async t => {
  let externalSystem;
  try {
    externalSystem = await startExternalSystem();
  } catch (error) {
    t.cancel();
    return;
  }

  // perform testing with `externalSystem`.
  t.is(typeof externalSystem, 'object');
});

So the questions I have - how should ava react if t.cancel() and t assertions are both run by the same test? In that case should order matter - t.true(true);t.cancel(); vs t.cancel();t.true(true);?

Also should the promise returned by the test matter to how t.cancel() is interpreted? In the example above the async function returns Promise.resolve(), which I think should cause the test to be reported as 'skipped'. What if t.cancel();return; were replaced with t.cancel();throw error;? Should the rejected promise be treated as an expected failure like t.failing()? Same question for a uncaught throw after t.cancel().

Sorry to bombard with questions, just trying to understand the edge cases.

novemberborn

novemberborn commented on Feb 20, 2019

@novemberborn
Member

I'd say you can only cancel if you haven't run assertions: Running assertions on a canceled test causes the test to fail. Canceling a test that has run assertions causes the test to fail.

We'll introduce a new reporting category for canceled tests.

tommy-mitchell

tommy-mitchell commented on Mar 2, 2024

@tommy-mitchell
Contributor

Now that assertions throw, would the return be necessary? I think that tests should still fail if they're cancelled after running assertions.

novemberborn

novemberborn commented on Mar 3, 2024

@novemberborn
Member

No, t.cancel() can also throw, just like a failed assertion.

I think if an assertion has already failed, then canceling has no effect. (To even be able to cancel you'd need to have caught the failed assertion error, which you're not supposed to do.)

A failed assertion after a cancellation (e.g. async) should also still fail the test IMHO.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @novemberborn@sindresorhus@coreyfarrell@tommy-mitchell

        Issue actions

          Feature Request: A way to mark a test 'skipped' after it starts · Issue #2010 · avajs/ava