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

describe.skip does not skip assertions in block #6233

Closed
dandv opened this issue Dec 4, 2024 · 3 comments · Fixed by #6237
Closed

describe.skip does not skip assertions in block #6233

dandv opened this issue Dec 4, 2024 · 3 comments · Fixed by #6237
Labels
bug Something isn't working needs triage

Comments

@dandv
Copy link
Contributor

dandv commented Dec 4, 2024

I expect that skipped describe blocks don't run any code inside, and especially not assertions.

Steps to Reproduce

  1. Create denobug.test.ts
import { describe, it } from '@std/testing/bdd';
import { assertExists } from 'jsr:@std/assert';

let skipTestingThisVariable: string;

describe.skip('skip everything in here', () => {
  assertExists(skipTestingThisVariable);  // <-- this line should be skipped
  it('should work', () => {
    assertExists(true);
  });
});
  1. Run deno test denobug.test.ts
Uncaught error from ./denobug.test.ts FAILED

 ERRORS 

./denobug.test.ts (uncaught error)
error: (in promise) AssertionError: Expected actual: "undefined" to not be null or undefined.
    throw new AssertionError(msg);
          ^
    at assertExists (https://jsr.io/@std/assert/1.0.8/exists.ts:29:11)
    at file:///~/deno/denobug.test.ts:7:3
    at new TestSuiteInternal (https://jsr.io/@std/testing/1.0.5/_test_suite.ts:91:23)
    at describe (https://jsr.io/@std/testing/1.0.5/bdd.ts:1111:22)
    at Function.describeIgnore [as ignore] (https://jsr.io/@std/testing/1.0.5/bdd.ts:1176:10)
    at Function.describeSkip [as skip] (https://jsr.io/@std/testing/1.0.5/bdd.ts:1208:19)
    at file:///~/deno/denobug.test.ts:6:10
This error was not caught from a test and caused the test runner to fail on the referenced module.
It most likely originated from a dangling promise, event/timeout handler or top-level code.

 FAILURES 

./denobug.test.ts (uncaught error)

FAILED | 0 passed | 1 failed (0ms)

error: Test failed

Expected behavior

The test should be ignored. If the assertExists(skipTestingThisVariable); is moved into the it, then it's correctly ignored

import { describe, it } from '@std/testing/bdd';
import { assertExists } from 'jsr:@std/assert';

let skipTestingThisVariable: string;

describe.skip('skip everything in here', () => {
  it('should work', () => {
    assertExists(skipTestingThisVariable);  // test is ignored now
    assertExists(true);
  });
});

Environment

  • deno version: 2.1.2
  • std version: "jsr:@std/expect@*": "1.0.8"
@dandv
Copy link
Contributor Author

dandv commented Dec 6, 2024

Thanks for the quick fix.

In the meantime I had a look at the same issue in Jest, and it's WAI:

Yeah, wording is imprecise - we will run the describe (so we can say test named a 1 is skipped), but we won't run any tests inside of it. If you have some setup that should be skipped within the describe, [p]ut it in beforeAll or beforeEach within the describe block.

Happy to take a PR specifying this in the docs 👍

I'm not advocating we keep Jest compatibility, but wondering if fixing this behavior might break something else.

Also, https://jsr.io/@std/testing/doc/bdd/~/describe.skip is somewhat vague about what "skip" does:

Registers a test suite with ignore set to true.

What exactly is that ignore?

What does "registering" means? Should some code inside still be... registered, just not run?

@IgorM867
Copy link
Contributor

IgorM867 commented Dec 6, 2024

@std/testing uses Deno test runner. Your code:

let skipTestingThisVariable: string;

describe.skip("skip everything in here", () => {
  assertExists(skipTestingThisVariable);
  it("should work", () => {
    assertExists(true);
  });
});

will be registered as:

Deno.test({ name: "skip everything in here", ignore: true }, (t) => {
  assertExists(skipTestingThisVariable);
  t.step("should work", () => {
    assertExists(true);
  });
});

So, ignore means the test will be skipped, and it will appear in the test report as ignored but will not be executed. Output:

skip everything in here ... ignored (0ms)

@dandv
Copy link
Contributor Author

dandv commented Dec 6, 2024

@IgorM867 thanks for the clarification. Given your knowledge of Deno test internals, I wonder if you could help bring BDD style testing to VS Code? This would be a breakthrough :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs triage
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants