You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Proposal: Move all the test in test/parallel into their own subsystem folder #16616
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.
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 #15437 (comment) 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)
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.
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.
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.
@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.
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 #15437 (comment), 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.
Reacted by Priyank Patel, Rich Trott and sreepurna
@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 :)
Activity
mscdex commentedon Oct 30, 2017
FWIW typically we've avoided such major churn in
test/.mscdex commentedon Oct 30, 2017
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 commentedon Oct 30, 2017
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 thinkingSubsystems:). 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 totools/test.pyto run tests for a given subsystem, like--subsystem=httpas #15437 (comment) 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 commentedon Oct 30, 2017
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 commentedon Oct 30, 2017
The subsystem metadata:
You can already run only (for example) http tests like this:
test.py httpWouldn'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
testin the filename.Trott commentedon Oct 30, 2017
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 commentedon Oct 31, 2017
@Trott it is documented in contributing.md and despite to submodules the tests have other tests like
test-cli-*or testtest-cluster-*which are not part of submodules.joyeecheung commentedon Oct 31, 2017
@Trott
The metadata idea is mainly to address #15437 (comment), which I believe is a valid argument against organizing tests into subsystem folders.
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 httpjust grabstest-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
I think this is true but
cliandclusterare valid subsystems(we have labels for them).test-regress-*and binding tests would be better examples.priyank-p commentedon Oct 31, 2017
@joyeecheung i think
Subsystem:comment is a good idea. Thanks for putting it in code-and-learn!sreepurnajasti commentedon Nov 3, 2017
@joyeecheung Agreed with adding
Subsystem:comment. Feels like it has its own pros.Its a great idea to put this in
code-and-learntasks. Great follow up :)priyank-p commentedon Dec 11, 2017
closing it now since i can work on it later until a month.