Support ignore: boolean for Other Test Definition

[ebebbington]

Description
It would be a huge help if the following test definition could support ignoring:

Deno.test("name", () => { ... })

// to
Deno.test("name", () => {
  ...
}, { skip: true }) // or something

The reason why i'm suggesting this is because of Rhum - we (after numerous days, albeit a while back) managed to finally complete integration tests for asserting output when using Rhum. Below is an example of how test output shows:

running 1 test; ...

src/server.ts
    run()
        Should start the server ... ok (200ms)

Now I am working on adding skip functionality, but Deno.test(name, fn) def doesn't allow it, meaning i have to use the object definition.

The problem with this is, i'm having to construct the output (programatically) in a different way to support skipping tests, so if test isn't skipped, do the usual, else modify the output so it's compatible with the object definition. Alongside this, it's been impossible to have working integration tests when using the object definition. The main reason is we use \n's and \u0008 when formatting the test names, which has been impossible to assert the actual output.

I know you already have 2 test definitions, but it would be a great help to Rhum to have both support ignore.

Other Notes
This is how we run a users' tests:

await Deno.test(c.name, async () => { // `c` = test case
  Deno.stdout.writeSync(encoder.encode(c.new_name)); // <-- actually what is displayed in the shell
  await hookAttachedTestFn();
});

But when using:

await Deno.test({
  name: c.new_name,
  ignore: skipTest,
  async fn(): Promise<void> {
    await hookAttachedTestFn()
  }
})

The raw output is completely different when using the obj def, because we have to adjust how to display the test output, meaning our assertions are wrong, and the formatting of the 'new name' needs to be re-written, which has proven near impossible to do

[nayeemrmn]

Ref #4351 (comment), I guess we can reconsider now. Yes, we would do this by adding a third options argument to the other overload:

export function test(
  name: string,
  fn: () => void | Promise<void>,
  options?: Omit<TestDefinition, "name" | "fn">,
): void;

[lucacasonato]

I don't see why... Deno.test(name, fn) is just sugar over Deno.test({name, fn}). Just use the object based syntax if you need more options. It is internally functionally equivalent.

[ebebbington]

The problem is i can't use the object based syntax, or i can but i'm unable to get Rhum working when doing so - we already have our integration tests working, and if ignore is added to (name, fn) then it's going to. make it 1000% easier to add the functionality to Rhum. Unfortunately it's already been a massive pain to test the Rhum output that makes the test output grouped into 'sections'

With adding the ability to skip tests, we're needing to replace c.name with c.new_name, eg:

// from
Deno.test({ name: c.name, async fn() { stdout.write(c.new_name) })

// to
Deno.test({name: c.new_name, async fn() { runTest() })

Though they aren't really the same as one does not allow options?

[nayeemrmn]

The supposed behaviour difference between the two overloads was a false alarm resolved in IRC. @ebebbington could you edit all of that out?

I also kind of wish we only had the object overload, and agree people should just use that. On the other hand, isn't it actually a simpler and slightly less opinionated design if both overloads supported the same option set? By having a third options argument which fills in those missing from the positional args.

[ebebbington]

Both are seen as the 'same', yet usage of them isn't (and i guess different internally because the string def doesnt pass anny of the options to the test runner?), and it seems odd to have two things that do the 'exact same' thing

@nayeemrmn edited my description 👍

[nayeemrmn]

@ebebbington I mean can you edit out everything starting with The reason why i'm suggesting this is because of Rhum because it's based on the false premise that we discussed already.

[ebebbington]

Realised adding ignore won't change anything for my use case. I'm happy toclose this issue as the description for the issue is tarnishing what seems like the actual issue of adding ignore, so it. can be closed, unless you wish to keep discussing the ignore options between yourselves

[ebebbington]

It would be nice if the deno test runner would allow itself or other modules to improve the output formatting - all in the name of readability, especially for large apps that are running dozens and dozens of tests, the output can be hard to read/structure