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 TestDefinition::skip #4098

Closed
nayeemrmn opened this issue Feb 23, 2020 · 10 comments · Fixed by #4351
Closed

Add TestDefinition::skip #4098

nayeemrmn opened this issue Feb 23, 2020 · 10 comments · Fixed by #4351
Assignees

Comments

@nayeemrmn
Copy link
Collaborator

Allow the following to disable a test:

Deno.test({
  skip: true, // <--
  name: "foobar",
  fn() {
    // ...
  }
});

Then uncomment all disabled tests in cli/js and std and skip them like this. That way the APIs used in disabled tests will forcibly be kept up to date (most of the time). Seen in #4092.

cc @bartlomieju

@ecyrbe
Copy link
Contributor

ecyrbe commented Feb 24, 2020

i like this.

@sholladay
Copy link

test.skip(name, fn) for shorthand style, please! This is how it works in AVA.

@ecyrbe
Copy link
Contributor

ecyrbe commented Feb 24, 2020

@sholladay This is not compatible with current syntax. The proposal don't disrupt current api. See test documentation.

@sholladay
Copy link

I could open a separate issue, I suppose. It's directly related, though.

People who don't use the object-based syntax (like me) don't want to have to start using it just to skip a test. It needs to support the shorthand form.

@nayeemrmn
Copy link
Collaborator Author

nayeemrmn commented Feb 25, 2020

If we want to apply this to the shorthand (which we currently have two of unfortunately), we can give it an options object containing the missing fields from TestDefinition:

export function test(t: TestDefinition): void;
export function test(fn: TestFunction, options?: Omit<TestDefinition, "name" | "fn">): void;
// ...

@sholladay
Copy link

sholladay commented Feb 25, 2020

Why not use the API of an existing popular testing library like AVA? Not only does that make things more familiar to users, but AVA already has high quality TypeScript type definitions that can be reused, which makes Deno development and maintenance easier. It's also a superior API in my opinion.

test.skip(name, fn);

@ry
Copy link
Member

ry commented Feb 25, 2020

I'm ok with test.skip(name, fn)

@kitsonk
Copy link
Contributor

kitsonk commented Feb 25, 2020

The only problem with that, is that it makes it harder to do a conditional skip. For example, you might want to skip on a particular OS for a time period:

Deno.test({
  skip: Deno.build.os === "win",
  name: "skip on windows",
  fn() { /* ... */ }
}

It is also easier to comment out or remove...

@sholladay
Copy link

sholladay commented Feb 26, 2020

I mean, you could use a simple ternary to do the same thing with function shorthand. I already destructure const { test } = Deno anyway, so it doesn't even add any lines for me, unlike the object form.

const testNix = Deno.build.os === 'win' ? Deno.test.skip : Deno.test;
testNix('skip on windows', () => { /* ... */ });

There are other possibilities here, too. With AVA, one can write a test macro to conditionally pass/fail any tests that use it.

They are also planning to implement t.cancel() to cancel a test during execution, which would look like this...

Deno.test('skip on windows', (t) => {
    if (Deno.build.os === 'win') {
        t.cancel();
    }
});

@kitsonk
Copy link
Contributor

kitsonk commented Feb 26, 2020

I said harder, not impossible.

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 a pull request may close this issue.

6 participants