tests: unified test names

[timreichen]

Is your feature request related to a problem? Please describe. test names in std use different implementation of test groupings ([brackets], [brackets/with/slashes], name with: colon, name with – dash) and casings. This was used before nested tests (t.step) were possible. These naming implementations are inconsistent amongst themselves, "pollute" the actual test name and can be replaced with nested tests or removed altogether.

This also will help newcomers in how to write and structure tests.

Describe the solution you'd like

This issue tracks steps to unify tests names in order to have a simple and consistent way of writing tests.

• [ ] remove bracket names and replace bracket nesting names with nested tests (https://github.com/denoland/deno_std/pull/2552)
• [ ] remove other string based nesting names with nested tests
• [ ] use same test name cases and use same syntax use lower case except mentions of actual classes and/or objects: Deno.test("do simething", …) but Deno.test("do something with BigInt", …)
• [ ] add entry for test name convention to the Contribution Guide

[iuioiua]

If we’re going to come up with a well-defined test name convention, it’d be worth detailing it in the contributing section.

[kt3k]

Let's not take action (like #2552) before we reached the consensus on how to write test names.

What do you think is the best way to name/organize test cases?

Is there any existing standard for it?

[timreichen]

There is https://github.com/mawrkus/js-unit-testing-guide#design-principles which collects a lot of good ideas in one document (except the "should" in every name which imo is obsolete) which could be a starting point and be adapted for our purposes.

[iuioiua]

Examples of my suggestion for a naming convention:

[fs] ensureDir() creates the directory if it doesn't exist
[async] delay() delays with abort option
[http] serveFile() responds with not found if the requested file doesn't exist

This convention is more or less used in parts of this repo. It aims to be clear and has three ingredients in order:

1. States the submodule
2. States the function or method in question
3. States the exact test scenario

[timreichen]

The goal would be not to have test names with "boilerplate", which means no mod name prefix an no function/method prefix. The mod name is given by the test file name which is logged automatically. And the function/method prefix should be the actual test name which contains steps that test the function/method. So it would look like this:

// fs_test.ts
Deno.test("ensureDir()", async (t) => {
  await t.step("creates the directory if it doesn't exist", () => ... })

})

The question is what name syntax/casings/tense phrase structure is to use for the name.

Do we use ensureDir or ensureDir() or ...

Do we use create the directory if it doesn't exist or creates the directory if it doesn't exist or the directory is created if it doesn't exist or ...

etc.

[iuioiua]

Actually, yeah, that's true. Having the module stated isn't needed, but it does help. I think ensureDir() wins over ensureDir just because it's more explicit. I think the snippet you shared looks good but the module name doesn’t hurt, IMO.

Do we use creates the directory if it doesn't exist or the directory is created if it doesn't exist or ...

I don't think this matters all that much.

@kt3k and @bartlomieju, what do you guys think?

[kt3k]

I think ensureDir() wins over ensureDir

I agree with this.

I think the snippet you shared looks good but the module name doesn’t hurt, IMO.

I'm in favor of keeping module names in test names. That might look verbose, but more obvious what are tested, less confusing to newcomers.

There are also cases where the test file name and exporting module name don't match. (For example encoding/json/_parse_test.ts tests APIs which are exported from encoding/json/stream. Without module names in test names, exporting module names are not obvious from test output.

[timreichen]

I'm in favor of keeping module names in test names. That might look verbose, but more obvious what are tested, less confusing to newcomers.

If it is easier to read in general, I think it makes much more sense to make deno test log a prefix based on the filename on every Deno.test name instead of this "workaround" by putting it into the name. I also like verbosity but not if data is doubled and needs to be copied all over. Apart from that does a test name only need to be verbose if a test fails, and in that case deno logs a link to the file and the corresponding line for direct access.

There are also cases where the test file name and exporting module name don't match. (For example encoding/json/_parse_test.ts tests APIs which are exported from encoding/json/stream. Without module names in test names, exporting module names are not obvious from test output.

In that case I think the file is not placed right and adding a prefix is a bit "hacky". As I have pointed out here std doesn't have a well defined file structure which needs to be resolved, which automatically would solve this particular issue:

• encoding
  • json
    • stream
      • _parse_test.ts

would print encoding/json/stream/_parse_test.ts which is clear imo.

[timreichen]

@kt3k It seems that the issues you raised have been solved: deno test always prints the test file name and we have now single export files and corresponding test files.

I think we could go forward in defining a convention now. @iuioiua what do you think?

[iuioiua]

I feel like we could spend the rest of time debating the test naming convention to use. Yet, it may not be clear which convention is best. Nevertheless, I think it should be kept as simple as possible. To reiterate, this is my idea of ideal test naming:

Examples of my suggestion for a naming convention:

[fs] ensureDir() creates the directory if it doesn't exist
[async] delay() delays with abort option
[http] serveFile() responds with not found if the requested file doesn't exist

This convention is more or less used in parts of this repo. It aims to be clear and has three ingredients in order:

1. States the submodule
2. States the function or method in question
3. States the exact test scenario

[timreichen]

I like that except the submodule name. Imo this should not be part of the test name because the submodule name has nothing to do with the test itself. It seems tedious to add the submodule name to each test, adding unnecessary duplicate information.

The test file path is printed along the test name, so

./fs/ensure_dir_test.ts => ensureDir() => creates the directory if it doesn't exist ... ok (31ms)

seems explicit enough to me. It also seems worth having a separator (for example =>) for between tested function/method/property and description I would suggest the following format:

[functionName]() => [description]
[className.method]() => [description]
[className.property] => [description]

=

./fs/ensure_dir_test.ts => ensureDir() => creates the directory if it doesn't exist ... ok (31ms)
./testing/bdd_examples/user_test.ts => User.getAge() => throws at undefined age... ok (19ms)
./testing/bdd_examples/user_test.ts => User.age => returns a number... ok (19ms)

[iuioiua]

@kt3k and I now agree that the above should be the test naming convention for the Standard Library. To reiterate, it's as simple as:

<symbol>() <description> // E.g. ensureDir() creates the directory if it doesn't exist

To do this, we can do the following:

• [ ] Create a separate tracking issue to tackle this change, one sub-module at a time. We don't want a single PR changing the entire codebase.
• [ ] Update the contributing guidelines. I can do this.

Does this all sound good @kt3k and @timreichen?

[timreichen]

@iuioiua this sounds great👍

[iuioiua]

Closing this in favour of #3754.