The test/ folder has grown organically. There are tests in there that form the "BCD linter" and there are now other files testing BCD scripts. We should organize ourselves a bit better there.
The linting scripts could live in a sub folder "linter" and be implemented consistently. These tests have probably quite a few things in common. Like they always have to walk the data and check it against the different rules and always spit out error messages. Can they share common functionality instead of re-implementing things?
Then there is test code that doesn't check data but rather tests for scripts we've started to add to BCD. Like traversing, stat generations, migration tests, and also the main bcd load function should be tested. I guess this stuff could be in subfolders, too, but not sure yet.
Let's draw a picture of how this could look better and then decide how to refactor this a bit. Post your proposals here before doing work! :-)
I think the linter folder would make sense.
There should probably also be the following files:
linter/index.js that would export an object containing all the current test‑*.js functions.linter/utils.js that would contain common test functionality:Logger object.Sounds good to me @ExE-Boss :+1:
I wonder if @vinyldarkscratch and/or @ddbeck have more thoughts on this before we proceed.
That sounds reasonable to me. My only request is that, to the extent it's practical, the changes happen incrementally and successively. It can be hard to review compound changes (e.g., moving to a new folder and also refactoring at the same time can be tough to follow), so I'd prefer a series of smaller PRs rather than one big-bang cleanup.
I’ve got no objections to that structure, looks good to me! The utils.js file is something I know we’ve had back-and-forth thoughts about, but I’m seeing lots more benefit to having it than not (I’m finding myself repeating some code at times).
For tests against our code, we should maybe also consider tools like Codecov, though that may be a more MDN-wide decision.
We started to organize ourselves a bit better now but there are still issues we need to look into. If you for example refactor or improve the linting scripts like in https://github.com/mdn/browser-compat-data/pull/4686, you have to manually verify it still does lint all the cases. I think it would make sense to write tests for the linting cases. So, right now we already moved the linter into its own directory:
test/linter/
I think it would be good to add tests and rename the lint scripts. For example like this:
test/linter/
Instead of a large PR that does all this, we could do this lint by lint and also require it for new linters.
Opinions on that?
This looks reasonable to me 👍
Most helpful comment
We started to organize ourselves a bit better now but there are still issues we need to look into. If you for example refactor or improve the linting scripts like in https://github.com/mdn/browser-compat-data/pull/4686, you have to manually verify it still does lint all the cases. I think it would make sense to write tests for the linting cases. So, right now we already moved the linter into its own directory:
test/linter/
I think it would be good to add tests and rename the lint scripts. For example like this:
test/linter/
Instead of a large PR that does all this, we could do this lint by lint and also require it for new linters.
Opinions on that?