discussion: adopt baretest-like testing API

[bartlomieju]

baretest is pretty interesting project that implements very simple testing framework (I wouldn't even call it a framework). It has a few functionalities which seem to be very desirable:

• beforeAll/afterAll hooks - can be used to setup expensive stuff before testing (DB connections, net connections, etc.)
• before/after hooks - can be used to implement useful sanity check, eg: verify that test closes all resources it opens; verify there are no pending ops when test finishes

Full code would look something like:

type TestHelper = () => void | Promise<void>;

export class TestSuite {
  _tests: TestDefinition[] = [];
  _only: TestDefinition[] = [];
  _skip: TestDefinition[] = [];
  _before: TestHelper[] = [];
  _after: TestHelper[] = [];
  _beforeAll: TestHelper[] = [];
  _afterAll: TestHelper[] = [];
  
  constructor(public name: string) {}
  
  test(name: string, fn: TestFunction): void {
    this._tests.push({ name, fn });
  }

  only(name: string, fn: TestFunction): void {
    this._only.push({ name, fn });
  }

  skip(name: string, fn: TestFunction): void {
    this._skip.push({ name, fn });
  }

  before(fn: TestHelper): void {
    this._before.push(fn);
  }

  after(fn: TestHelper): void {
    this._after.push(fn);
  }

  beforeAll(fn: TestHelper): void {
    this._beforeAll.push(fn);
  }

  afterAll(fn: TestHelper): void {
    this._afterAll.push(fn);
  }
}

And API:

const s = new Deno.TestSuite("my test suite");

let resourceSnapshot;
s.before(function () {
   resourceSnapshot = Deno.resources();
});

s.after(function () {
   const resources = Deno.resources();
   // diff with `resourceSnapshot` and ensure test
   // doesn't leak resources
   assertEquals(resourceSnapshot, resources, "test leaks resources");
});

s.test(function foobar() {
  ...
});

// TODO: fixme, temporary disabled
s.skip(function fizzbuzz() {
  ...
});

// Can also be run directly by `deno test` subcommand
await s.run();

Ref #4090

[bartlomieju]

Just to clarify - Deno.test() would still exist.

This issue is about addition of new Deno.TestSuite() which makes built-in testing useful for both small or larger projects; while still keeping implementation dead simple.

[nayeemrmn]

It would be nice if we could add these hooks while still using Deno.test(), closer to the way we do it now. It would take some strategy to be able to specify the scopes of the before()s and after()s, though.

Here's an idea:

let resourceSnapshot;
const s = new Deno.TestScope("my test suite", {
  beforeEach() {
    resourceSnapshot = Deno.resources();
  },
  afterEach() {
    const resources = Deno.resources();
    // diff with `resourceSnapshot` and ensure test
    // doesn't leak resources
    assertEquals(resourceSnapshot, resources, "test leaks resources");
  }
});

s.start();

Deno.test({
  name: "foobar",
  fn() {
    // ...
  }
});

// TODO: fixme, temporary disabled
Deno.test({
  skip: true,
  name: "fizzbuzz",
  fn() {
    // ...
  }
});

s.end();

[ecyrbe]

Maybe i'm confused, but what about making Jest deno compliant ? is it too much work ?

[bartlomieju]

@nayeemrmn s.start() and s.end() seem very funky are error-prone. I believe TestSuite is pretty common term in testing.

@ecyrbe Definitely; Jest has >50k lines of code and has >70 deps which are mostly Node specific. Besides it's full-blown testing framework with snapshots, mocking, etc.

[kevinkassimo]

Bringing these hooks to Deno itself is fine I believe since they are truly minimal.

@nayeemrmn I think your proposal could be confusing, since there is no clear association between Deno.test() with the TestSuite instance s, and using s.start()/end() marker does not provide any enforcements pre-runtime. (and it is just so easy to forget to call s.end())

@ecyrbe Jest is just too huge a framework to port. To be honest I would rather put such effort to std/node polyfills than for Jest itself :) (maybe with enough Node polyfills we can get it work)

[nayeemrmn]

@bartlomieju How about giving it as an option to Deno.test():

let resourceSnapshot;
const suite = new Deno.TestSuite("my test suite", {
  beforeEach() {
    resourceSnapshot = Deno.resources();
  },
  afterEach() {
    const resources = Deno.resources();
    // diff with `resourceSnapshot` and ensure test
    // doesn't leak resources
    assertEquals(resourceSnapshot, resources, "test leaks resources");
  }
});

Deno.test({
  name: "foobar",
  suite,
  fn() {
    // ...
  }
});

// TODO: fixme, temporary disabled
Deno.test({
  skip: true,
  suite,
  name: "fizzbuzz",
  fn() {
    // ...
  }
});

Or have Deno.test() return some kind of symbol and giving those as a list to new TestSuite():

let resourceSnapshot;
const suite = new Deno.TestSuite("my test suite", {
  beforeEach() {
    resourceSnapshot = Deno.resources();
  },
  afterEach() {
    const resources = Deno.resources();
    // diff with `resourceSnapshot` and ensure test
    // doesn't leak resources
    assertEquals(resourceSnapshot, resources, "test leaks resources");
  },
  tests: [
    Deno.test({
      name: "foobar",
      fn() {
        // ...
      }
    }),

    // TODO: fixme, temporary disabled
    Deno.test({
      skip: true,
      suite,
      name: "fizzbuzz",
      fn() {
        // ...
      }
    })
  ]
});

Too many indents, probably. These are just examples of strategies to make Deno.test() the only way of declaring a test. I think that's important.

[kevinkassimo]

@nayeemrmn IMHO I don't think adding a separate Deno.TestSuite can be that much of a cost. If we were to really only forced to maintain a single set of API, I would rather have Deno.TestSuite.default.test(), maintaining a static instance of TestSuite instead of keeping Deno.test.

Actually, I am actually thinking about making Deno.test() an alias to Deno.TestSuite.[defaultInstance].test(), and have things like Deno.test.beforeAll()/.afterAll()/.... In that sense, the tests declared through Deno.test() are implicitly on a default instance of TestSuite.

[ecyrbe]

@bartlomieju Another silly question : so what about mocha + jspm as mocha does not rely on node and can run in the browser, and deno is almost like a browser : https://mochajs.org/#running-mocha-in-the-browser

[bartlomieju]

@ecyrbe you can already run mocha with Deno; example in typeorm port for Deno: https://github.com/denolib/typeorm/blob/master/test/deps/mocha.ts

[ecyrbe]

@bartlomieju So we already have access to mocha hooks ? why not use them ?

[kevinkassimo]

@bartlomieju Actually I just noticed that deno is panicking if you try to import https://unpkg.com/mocha/mocha.js due to redirect to an absolute URL: https://unpkg.com/mocha/mocha.js redirects to /[email protected]/mocha.js, which cannot be parsed correctly. I'll see if I can do a quick fix for this.

[kevinkassimo]

@ecyrbe Deno core should offer a set of bare minimal utilities, instead of being too opinionated IMO

[ecyrbe]

@bartlomieju Thanks, that clarifies everything.

So i guess what deno expose today is sufficiently deadly simple and maybe we should orient users to use more complex libraries when needing advanced testing functionnality.

What i dislike here is adding another way to write deno native tests. If hooks are really necessary, i would then advise going all in and remove Deno.test() api.

[lucacasonato]

What i dislike here is adding another way to write deno native tests. If hooks are really necessary, i would then advise going all in and remove Deno.test() api.

I agree - we should either provide only the dead simple Deno.test, or Deno.TestSuite, but not both as they don't work together - that would create confusion. I would personally vote for some sort of Deno.TestSuite, but I don't think a default instance is a good idea, because it might be confusing for new users that all Deno.test (or Deno.TestSuite.default.test() <- very long btw) are shared between modules. Also would you have to manually run() the default instance?

[kitsonk]

Personally, I don't feel we need to introduce a class to make all this happen.

How are capabilities like test setup and teardown work in Rust? That model seems to be very effective for us. I believe there is effectively block of code that are only included in the final binary and run when you do a test build, and the implementation of anything fancy is left up to the implementor.

I think we should really think minimalistic, otherwise we will be in the test harness business, not the JavaScript/TypeScript runtime business.

[ry]

I agree with @kitsonk that we don't need setup/teardown. Rust's unit tests are a great example of this - they provide no such facility - and yet we're able to build very complex software on top of it.

We will not be introducing a more complex test facility for Deno. It should be simply registering functions to be run and checking if they get exceptions.

[keroxp]

I agree that keeping it minimal, but as beforeAll and afterAll, we actually need them for std tests. Some integration tests certainly needs them for managing external processes / server. Currently there are no way to cleanup tests after test run. IMO Rust's test system is designed only for functional unit test.

I think Go is resolving the problem well by providing run hook.

func setup() {
    println("setup")
}

func teardown() {
    println("teardown")
}

func TestMain(m *testing.M) {
    setup()
    ret := m.Run()    
    teardown()
    os.Exit(ret)
}

[hayd]

You can write your own. I have used something like:

export async function test(t: Deno.TestDefinition) {
  async function wrapped() {
    await setUp();
    try {
      await t.fn();
    } finally {
      await tearDown();
    }
  }
  Deno.test({ name: t.name, fn: wrapped });
}

Perhaps something to aid that could be in std, but it's not necessary in Deno namespace.

[oscarotero]

I have reached to this issue searching for support for beforeEach and afterEach. I understand the reason why you don't want to implement a new Deno.TestSuite() (or similar API). But maybe a more simple way to implement this is by extending the current Deno.test() allowing an array of tests and adding the setup and teardown functions to the TestDefinition. For example:

Deno.test({
  name: "example test",
  fn: [
    function test1() {
      assertEquals("hello", "hello");
    },
    function test2() {
      assertEquals("world", "world");
    }
  ],
  beforeEach() {
    // Code to execute before each test function
  },
  afterEach() {
    // Code to execute after each test function
  },
  beforeAll() {
    // Code to execute before all test functions are executed
  },
  afterAll() {
    // Code to execute after all test functions are executed
  }
});

[timreichen]

How about adding nested Deno.test() and await Deno.test() support?

Deno.test("suite", () => {
  // beforeAll
  const tests = [
    {
      name: "test1",
      fn: () => {
        …
      }
    },
    {
      name: "test2",
      fn: () => {
        …
      }
    }
  ]
  
  for await (const test of tests) {
    // beforeEach
    await Deno.test(test)
    // afterEach
  }
  // afterAll
})

console:

test suite
  test test1 ... ok
  test test2 ... ok

That way we would be able to implement the upper described functionality without adding any new syntax.

[nayeemrmn]

@timreichen I think that's a really nice solution :-) Can you open a new issue? Use the code example more like the one here: https://discord.com/channels/684898665143206084/684911491035430919/762300901590433812.

[KyleJune]

Here is a wrapper I implemented for adding test setup/teardown to Deno.test. I based it on the idea of having a TestSuite from this discussion. I didn't make an issue suggesting it because it seemed to already be decided that deno doesn't need setup/teardown built in.

https://deno.land/x/test_suite