Skip: Show Ignored in Output?

[ebebbington]

Note: it might not be possible with how. we use Deno.test. Maybe we could add custom output outselves to say it was skipped (but it might still show as OK), or (and i really dont want to do this), change our Deno.test signature to Deno.test({name: ..., ignore: ..., async fn() ...})

Summary

What: Modify the skip method in RhumRunner so it stills calls Deno.test, but ignore: true is passed in. This would probably require a change to ITestCase to set if a test should be ignored. Defaults to false

Why: I think it would be good that the user would see the test is being ignored (like now deno test currently displays it)

Acceptance Criteria

• Skipped tests would show as ignored in the output (all tests inside a skipped suite, all tests inside a skipped plan, etc)
• Unit tests for .skip method
• Integration test for using skip: tests/integration/skip.ts

Example Pseudo Code (for implementation)

// run()

Deno.test(..., { ignore: test.ignore })

// skip
public skip (name, fn) {
  if (this.is_a_test) {
    this.tests.push(..., ignore: true)
  else if this.passed_in_test_suite // skipped a suite
    this.skipped_suites.push(name)
  else // plan
    this.skip_plan = true

// somewhere else
this.plan.suites.foreach
  suite.cases.foreach
    if this.skipped_suites.indexOf(suite.name)
      case.ignore =. true
    if this.skip_plan === true
      cas.eignore = true

[crookse]

I like this

[Guergeiro]

Just a thing that would be interesting, to show the message as Skipped instead of Ok/Error. It would still count as a pass though.

[ebebbington]

Just a thing that would be interesting, to show the message as Skipped instead of Ok/Error. It would still count as a pass though.

Might be possible, like we could add colours.yellow("Skipped") after the end of the new-name?

It might look like this:

server.ts
    run()
        It should run the server ... skipped ok (2ms)

[ebebbington]

Working on it, and @Guergeiro this is how it displays, without me doing colours.yellow(...):

We could probably just leave it like that, as it actually already shows it's ignored (but not "skipped")

[Guergeiro]

Working on it, and @Guergeiro this is how it displays, without me doing colours.yellow(...):

We could probably just leave it like that, as it actually already shows it's ignored (but not "skipped")

I'm okay with being ignored instead of skipped. But if it's displaying yellow without we telling it to be yellow, it means we are depending on Deno, and we all know how Deno is breaking things left and right. I would force our yellow colour.

[ebebbington]

Hmm... idk, because by that, we may as well do the same for "ok" and "FAILED", as these display green/red without us doing anything

So i'd probably have to disagree as i think it add unnecessary work? Should the time arise where they remove the colours we could address it then? Though unsure how we would do that as of now as we rely on the deno test runner to display the "ok/FAILED/ignored"

[Guergeiro]

Yeah, that's a good point. Let's leave it at that.

[ebebbington]

It'd be great is we could have more control though 👍

[ebebbington]

Don't know if it'll be possible to do Rhum.skip, as we have no context to figure out if the skipped 'thing' is a suite or test case, for example take the following:

import {Rhum} from "../../mod.ts";

Rhum.testPlan("example_tests/skip/skipped_case", () => {
  Rhum.testSuite("doSomething()", () => {
    Rhum.testCase("Returns something", () => {
      Rhum.asserts.assertEquals(true, true) // test  should be skipped so this should never fail
    })
    Rhum.testCase("Returns something else", () => {
      Rhum.asserts.assertEquals(true, true)
    })
  })
  Rhum.skip("doSomethingElse()", () => {
    Rhum.testCase("Returns another thing", () => {
      Rhum.asserts.assertEquals(true, false)
    })
  })
})

Rhum.run()

Where you see Rhum.skip, how do we know if it is a suite or not? Below is the current object when the method is called:

public skip(name: string, cb: Function): void {
    console.log(this)
    ...
}

RhumRunner {
  asserts: {
    ...
  },
  passed_in_test_plan: "example_tests/skip/skipped_case",
  passed_in_test_suite: "doSomething()",
  test_plan_in_progress: "example_tests/skip/skipped_case",
  test_suite_in_progress: "doSomething()",
  plan: { suites: { doSomething(): { cases: [Array], skip: false } }, skip: false },
  mocks: { ServerRequest: [Function: MockServerRequestFn] }
}

I can't think of a way to figure it out, so i'm thinking along the lines of maybe doing. something like:

Rhum.testPlan("name", ({ skip: true }) => {
    ...
})
// or
Rhum,testPlan("name", () => {
    ...
}, { skip: true })

[ebebbington]

Worked on it, but giving up as it's too time consuming to fix the assertions. In a nutshell:

• I did this to the skip method:

if (this.passed_in_test_plan && this.passed_in_test_suite) { // is a test case being skipped
      this.plan.suites[this.passed_in_test_suite].cases!.push({
        name,
        new_name: this.formatTestCaseName(name, true),
        testFn: cb,
        skip: true
      });
    } else if (this.passed_in_test_plan) { // is a test suite being skipped
      this.passed_in_test_suite = name;
      this.plan.suites![name] = { cases: [], skip: true };
      cb();
    } else {
      // is a test plan being skipped
      this.passed_in_test_suite = ""; // New plan
      this.passed_in_test_plan = name;
      this.plan.skip = true;
      cb();
    }

• I added skip to ITestPlan, ITestSuite and ITestCase

• We would have to use the object definition for skipping tests, which changes the way our output gets displayed (hence the time consuming bit)

• I pretty much did this inside of the method that runs the tests:

const skipTest =
            c.skip === true || this.plan.skip === true || this.plan.suites[suiteName].skip === true
        if (Deno.env.get("CI") === "true") {
            await Deno.test({
              name: c.new_name,
              ignore: skipTest,
              async fn(): Promise<void> {
                await hookAttachedTestFn()
              }
            })
        } else {
          await Deno.test({
            name: c.new_name,
            ignore: skipTest,
            async fn(): Promise<void> {
              //Deno.stdout.writeSync(encoder.encode(c.new_name))
              await hookAttachedTestFn()
            }
          })

• Formatting of the new_name would have to be adjusted

[ebebbington]

Made an issue for Deno in hopes of getting ignore for the test definition. we. use: denoland/deno#7173