Node: Proposal: Move all the test in test/parallel into their own subsystem folder

[priyank-p]

Currently there are 1618 file in test/parallel so if we move all the test files into their subsystem folder it would be more organized.

test/parallel/
|
| --- assert/
| ---- test-assert-*.js
|
| --- repl/
| ---- test-repl-*.js
|

[mscdex]

FWIW typically we've avoided such major churn in test/.

[mscdex]

I should also add that we've had this particular discussion before (I cannot recall the PR/issue) and for one thing it's not as cut-and-dry as you suggest. There are many tests that test the integration/behavior of multiple modules.

EDIT: Here is the previous PR on this topic.

[joyeecheung]

Idea on an easier transition: let's figure out what subsystems these tests are touching and add a formatted comment there (like the Flags: that we have, I am thinking Subsystems:). When we done annotating all the tests, it would be easier to know if this is the right direction, with the benefit of better documented tests. (Also with that we can do things to tools/test.py to run tests for a given subsystem, like --subsystem=http as https://github.com/nodejs/node/pull/15437#issuecomment-330050638 suggests).

This could be added to the code-and-learn tasks (not necessarily alone, we just need to ask attendees to add a comment while they are changing the code)

[joyeecheung]

Also I think we can ask the code-and-learn participants to add a comment to the test describing what it tests if there isn't one and they feel confident enough to actually dig into the test.

[Trott]

The subsystem metadata:

• You can already run only (for example) http tests like this: test.py http

• Wouldn't surprise me if the above is undocumented. That should certainly be fixed if that's the case.

• Subsystems requirement should be added to the guide on writing tests if it's something we want each test to have.

• I can imagine some people perhaps thinking we shouldn't formalize additional metadata unless we know we are going to use it and it will be high value.

• I can also imagine people arguing that since the subsystem is right there in the file name most of the time, the thing to do is rename files where the primary subsystem isn't the first thing after test in the filename.

[Trott]

Oh, to be clear: I'm totally not sure how feel about the subsystem metadata. I'm not arguing for it or against. Just providing immediately thoughts and hopefully some useful additional information.

[priyank-p]

@Trott it is documented in contributing.md and despite to submodules the tests have other tests like test-cli-* or test test-cluster-* which are not part of submodules.

[joyeecheung]

@Trott

You can already run only (for example) http tests like this: test.py http
I can imagine some people perhaps thinking we shouldn't formalize additional metadata unless we know we are going to use it and it will be high value.
I can also imagine people arguing that since the subsystem is right there in the file name most of the time, the thing to do is rename files where the primary subsystem isn't the first thing after test in the filename.

The metadata idea is mainly to address https://github.com/nodejs/node/pull/15437#issuecomment-329974923, which I believe is a valid argument against organizing tests into subsystem folders.

I have a feeling this structure will cause confusion for the tests we have that involve multiple modules (e.g. cluster and dgram). How would you know where to put those tests? If you put them in the wrong directory, you run the risk of missing them when running the other group of tests.

We have quite a few tests testing the interoperability of different modules e.g. fs and whatwg-url, async hooks and a lot of other modules. IIUC, test.py http just grabs test-http-* but when a test is testing multiple subsystems, it might be hard to tell from the filename or putting them all there would make the name too long (order the subsystems in alphabetical order might help tho).

I believe a subsystem comment would help us understand how many tests like this we have right now, but yes I understand the concern of legacy formatted metadata that are not really used anywhere. Still I believe it worths a try.

@cPhost

it is documented in contributing.md and despite to submodules the tests have other tests like test-cli-* or test test-cluster-* which are not part of submodules.

I think this is true but cli and cluster are valid subsystems(we have labels for them). test-regress-* and binding tests would be better examples.

[priyank-p]

@joyeecheung i think Subsystem: comment is a good idea. Thanks for putting it in code-and-learn!

[sreepurnajasti]

@joyeecheung Agreed with adding Subsystem: comment. Feels like it has its own pros.
Its a great idea to put this in code-and-learn tasks. Great follow up :)

[priyank-p]

closing it now since i can work on it later until a month.