Use subtests for esbuild, mdx, and remark-mdx

[remcohaszing]

These packages have significantly sized test cases. Using subtests helps when troubleshooting these tests.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
mdx	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jul 16, 2023 10:13am

[codecov-commenter]

Codecov Report

Patch and project coverage have no change.

Comparison is base (ee176d7) 100.00% compared to head (f94d2aa) 100.00%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files

@@            Coverage Diff            @@
##              main     #2327   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           23        23           
  Lines         2246      2246           
  Branches         4         4           
=========================================
  Hits          2246      2246           

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[wooorm]

Looks like a couple cases earlier on have a capitalized, different message, while the assertion message is still in place. I think you can move that message to the test case (so that it prints like “should ...”). Then it’s TLDR but fine!

[ChristianMurphy]

Thanks @remcohaszing!
It might be even nicer to use describe and it("should ...") for suites and tests, but that is nice to have.
This in and of itself is a good improvement. 👍

[wooorm]

I’m personally not much of a fan of describe and it. AFAIK they don’t add anything really, and using test only seems just simpler?

[remcohaszing]

I like using describe blocks, but chose subtests because @wooorm has expressed he prefers those in another issue somewhere. It’s mostly a stylistic difference.

[ChristianMurphy]

AFAIK they don’t add anything really

They add clarity, by being closer to natural language.
describe clarifies this is a label of what we are testing.
it naturally flows into test description beginning with "should ...".

and using test only seems just simpler?

some test actually test things, other test don't test anything and act only as a label, that feels more confusing to me.

Again, this is not a blocker.

[wooorm]

Thanks Remco! Appreciate the work! <3

[wooorm]

@ChristianMurphy

They add clarity, by being closer to natural language.

I don’t really buy into this argument; it sounds nice, but if that’s the case, why don‘t we do that in other places as much as tests?

some test actually test things, other test don't test anything and act only as a label, that feels more confusing to me.

It used to be that there was a clearer divide between test as the whole wrapper, and assert for the actual testing things. Remco pointed out that he prefers wrapping assertions into separate test cases so that tests keep running if one assertion fails, and that the output is nicer (you see a bunch of green stuff for what’s good, red stuff for what isn’t).
These are differences where tape was different from node:test. Although I find it noisy to have so many test cases, I think they’re fine trade-offs, dropping a dev-dep and switching to the built in node:test is nicer.

I feel like that’s more another layer of tests grouping things; assertions still do the only “actual testing”: all tests are just groupings and labels.

[ChristianMurphy]

why don‘t we do that in other places as much as tests?

That's a good question, why not?
I don't really buy into "we have unclear naming already, so let's keep doing it"

These are differences where tape was different from node:test. Although I find it noisy to have so many test cases, I think they’re fine trade-offs, dropping a dev-dep and switching to the built in node:test is nicer.

agreed

I feel like that’s more another layer of tests grouping things; assertions still do the only “actual testing”: all tests are just groupings and labels.

I agree there is another layer of grouping.
I feel like spec style testing (describe and it) fits better with unified's culture of coding to spec.
And that the main reason it isn't already in place, is that tape didn't support this convention for testing.

[wooorm]

That's a good question, why not?

My personal answer would be: because programming is not natural language. Trying to merge them doesn’t work well. But that’s from my experience.

And that the main reason it isn't already in place, is that tape didn't support this convention for testing.

I used to use it https://github.com/retextjs/retext/blob/1.0.0/test.js, jasmine/mocha, I personally preferred t style over describe/it-style!

I feel like spec style testing (describe and it) fits better with unified's culture of coding to spec.

This is interesting! Why?

[ChristianMurphy]

My personal answer would be: because programming is not natural language. Trying to merge them doesn’t work well. But that’s from my experience.

I'm not saying they are precisely the same, nor that we should merge them.
We do aim to name things sensibly in English, with names that follow convention so they are easily understood.
Spec driven development naming is one such convention.

This is interesting! Why?

It by naming convention and design convention focuses on, the why and what of how a piece of code should work.
Over a standard test driven approach, which aims to hit 100% coverage, without much consideration to why specific features or edge cases are needed.

[wooorm]

We do aim to name things sensibly in English, with names that follow convention so they are easily understood.

Important distinction is of course, sure, we use natural language, but trying to make a running English sentence with code is something else.

To me, test('should parse a self-closing tag', … reads as clearly as it('should parse a self-closing tag', …. But having one name for grouping things, test, seems easier.