Less verbose test function

[MarkTiedemann]

Currently, the test function has the following usage:

// #1

test({
  name: "example",
  fn() {
    // test code
  }
});

// #2

test(function example() {
  // test code
});

I like #2, but I think #1 is a bit verbose.

How about this instead?

test("example", () => {
  // test code
});

[kitsonk]

Personally, I really hate overloads that change the ariaty of the arguments. Omitting "optional" parameters is different. Adding complexity to a straight forward API without real value doesn't seem warranted IMO.

[MarkTiedemann]

Well, yeah, that would change the arity of the function. Before, however, the arity was kind of "hidden" in the object properties. In both cases, two things (the name and a function) need to be passed to the test function.

I still think an options object with two specifically named properties is more complex than two parameters... But it might also be that it's just more familiar to me since quite a lot of JS testing frameworks (for example, mocha and ava) have chosen this approach.

[hayd]

See also denoland/deno#20

[keroxp]

I think It is good idea.

I still think an options object with two specifically named properties is more complex than two parameters...

I agree. I rarely saw test runner written in that style, name and fn look redundant.
It is natural to define test() with 2 argument: description, body for most JS developers. It's usual style in Jasmine, mocha, RSpec..

As @kitsonk said, I know defining typed function with different number of arguments is quite complicated. But It can be by TypeScript. I understand this may not be the best, but believe a great and flexible feature of TS(JS).

Patches will be like this:

export type TestDescription = {
  (description: string, body: TestFunction)
  (body: TestFunction)
}

export const test: TestDescription = function test (descOrBody: string|TestFunction, body?: TestFunction): void {
  let name: string;
  let fn: TestFunction;
  if (typeof descOrBody === "string") {
    name = descOrBody;
    fn = body;
  } else {
    name = descOrBody.name;
    fn = descOrBody;
  }

  if (!name) {
    throw new Error("Test function may not be anonymous");
  }
  if (filter(name)) {
    tests.push({ fn, name });
  } else {
    filtered++;
  }
}

Obviously, more discussions will be needed🧐

[MarkTiedemann]

I think the correct overloaded typings are:

declare function test(fn: () => void): void;
declare function test(name: string, fn: () => void): void;

Which could be used as follows:

test(function example() { /* test code */ })
test('example', () => { /* test code */ })

Unfortunately, I don't think it's possible to distinguish named and anonymous functions in TypeScript... so that's gotta be a runtime fn.name check.

[MarkTiedemann]

The implementation of the overloading might not look all too pretty.

export function test(fnOrName: (() => void) | string, undefOrFn?: () => void) {
  if (typeof fnOrName === "function") {
    if (fnOrName.name === "") {
      throw new Error(`'fn' must be a named function`);
    } else if (undefOrFn !== undefined) {
      throw new Error(`second parameter must be 'undefined'`);
    } else {
      let fn = fnOrName;
      let name = fnOrName.name;
      // Yay!
    }
  } else if (typeof fnOrName === "string") {
    if (typeof undefOrFn !== "function") {
      throw new Error(
        `'fn' must be typeof 'function', but found '${typeof fnOrNone}'`
      );
    } else {
      let fn = undefOrFn;
      let name = fnOrName;
      // Yay!
    }
  } else {
    throw new Error(
      `first parameter must be typeof 'function' or 'string', but found '${typeof fnOrName}'`
    );
  }
}

But using the API is more intuitive, IMHO.

[MarkTiedemann]

@ry Would you accept a PR that implements this breaking change? If so, I'd start working on it.

[ry]

@MarkTiedemann Seems reasonable to me. Go for it.

[hayd]

I dislike overloading this much further...

I think it's going to be much more useful to add setup and teardown, an example might be something like this: hayd@987ef51

[keroxp]

@MarkTiedemann Any progress? I want see it 🧐.
@hayd @kitsonk If you don't like definition overload, let us consider defining test function with exactly 2 arguments? We have overload for a while, and deprecate early style of test in the future.

[MarkTiedemann]

@MarkTiedemann Any progress? I want see it 🧐.

Not yet, unfortunately. I was working on deno_install in the past week.

My next week is gonna be busy, too, so if you want to work on it, feel free!

I think it's going to be much more useful to add setup and teardown

I guess that's a matter of taste.

I personally don't like it when test frameworks offer a special way to do setup and teardown (which you have to learn or look up in the docs).

I prefer a plain old function call for setup and a try-finally-block with a function call for teardown. For example:

test(function exampleTest() {
  mySetupCode()
  try {
    myTestCode()
  } finally {
    myTeardownCode()
  }
})

In this case, you don't have to know the test framework. This works everywhere JS works. It's plain and simple.

Also, not many test cases actually require custom setup or teardown code, whereas all test cases require a name. So if I had to choose between those features, as you seem to imply, I'd go with the less verbose naming feature rather than the setup and teardown feature.

[hayd]

See also #128.

[sholladay]

What do y'all think about AVA's API?

Personally I love it and it already has great TS types, so should be very easy for Deno to adopt.

[ry]

If someone ported AVA to deno_std and it worked perfectly, I would be happy to have it. But likely that won't happen - because it looks big and complex.

I'm a fan of the current testing module because of how minimal it is. I realize it has some problems at the moment (#152, #162, #122), but I intend to clean it up soon. I think it's worthwhile to have a very minimal std testing module.

[sholladay]

Sorry, I meant the API definition (or a small subset of it), not necessarily the implementation. Would be awesome if AVA's implementation could be ported, but I agree that sounds hard. Among other things, AVA relies on Node's child_process. It might be possible to replace that with Web Workers, but there could be other issues.

If, however, Deno were to adopt the API of a popular framework like AVA, it would make it easier to port the implementation in the future as better tooling comes along to do that, and as libraries begin to use more runtime agnostic patterns. At the very least it would make Deno more familiar and approachable.

The similarities don't have to go super deep, IMO, as long as test definitions and assertions are the same.

[ry]

@sholladay Hm - looks quite minimal and similar to ours. I'm not opposed to that API.

@piscisaureus thoughts? https://github.com/avajs/ava/blob/master/docs/01-writing-tests.md