Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

☂️ Move Astro tests to node:test #9873

Closed
19 tasks done
ematipico opened this issue Jan 30, 2024 · 43 comments · Fixed by #10142
Closed
19 tasks done

☂️ Move Astro tests to node:test #9873

ematipico opened this issue Jan 30, 2024 · 43 comments · Fixed by #10142
Labels
- P1: chore Doesn't change code behavior (priority) good first issue Good for newcomers. If you need additional guidance, feel free to post in #dev on Discord

Comments

@ematipico
Copy link
Member

ematipico commented Jan 30, 2024

Description

Internally, the team decided to move our testing infrastructure to use node:test instead of Mocha.

While Mocha is good, it has its own quirks. Also, we believe that the Node.js test runner will improve with time.

This is an excellent opportunity to contribute to Astro and help us with the migration because you don't need to understand Astro logic. So if you have been looking for an opportunity to contribute to Astro, this is the perfect chance for you!

Plan

Comment here which integration/adapter you want to migrate first. Failing to comment might nullify your contribution if someone else did the work and commented on the issue. This will help us to avoid having multiple people working on the same thing.

The plan should be as follows:

  1. Migrate tests of our @astrojs/* packages. This can be done in one PR, because we only have a few tests inside our integrations/adapters. You can use this PR as a blueprint of what we're looking for: chore(@astrojs/node): use Node.js for testing #9758
  2. Progressively migrate the tests we have in core, starting with the unit tests, then integrations tests. This means that we need to have two scripts, one that will run the tests using Mocha, and one that will run the tests using Node.js

Integrations/adapters

List of the integrations/adapters

Core

astro

After #10002 is merged, we can progressively start migrating tests.

Instructions:

  • rename a *.test.js file to *.nodetest.js, e.g. api.test.js -> api.nodetest.js
  • migrate the content of the test to use Node.js test runner
  • use the command pnpm test:node to run only the files that use Node.js test runner

Warning

Node.js test runner doesn't handler deep nested describe, so it's possible your test could hang and timeout. If that happens, lift the describe function.

External adapters

withastro/adapters#144

@github-actions github-actions bot added the needs triage Issue needs to be triaged label Jan 30, 2024
@ematipico ematipico added - P1: chore Doesn't change code behavior (priority) good first issue Good for newcomers. If you need additional guidance, feel free to post in #dev on Discord and removed needs triage Issue needs to be triaged labels Jan 30, 2024
@ematipico ematipico changed the title Move Astro tests to node:test ☂️ Move Astro tests to node:test Jan 30, 2024
@ematipico ematipico pinned this issue Jan 30, 2024
@log101
Copy link
Contributor

log101 commented Jan 30, 2024

Would love to contribute, I'd like to grab @astrojs/tailwind if that's OK?

@mingjunlu
Copy link
Contributor

mingjunlu commented Jan 30, 2024

I would like to migrate tests of @astrojs/sitemap 🗺️

@VoxelMC
Copy link
Contributor

VoxelMC commented Jan 30, 2024

I'll try first with telemetry!

I know it's a core package, but it is a small one, so I thought it could start some discussion about progressively migrating the tests!

@at-the-vr
Copy link
Contributor

I will take a hit on @astrojs/vue

@alexnguyennz
Copy link
Contributor

Working on @astrojs/mdx.

@ktym4a
Copy link
Contributor

ktym4a commented Jan 31, 2024

I will take @astrojs/markdoc.

@VoxelMC
Copy link
Contributor

VoxelMC commented Jan 31, 2024

I will do @astrojs/react!
Does @astrojs/svelte have tests?

@ematipico
Copy link
Member Author

I will do @astrojs/react!
Does @astrojs/svelte have tests?

It seems not, thank you for checking

@xMohamd
Copy link
Contributor

xMohamd commented Jan 31, 2024

I will do @astrojs/react!
Does @astrojs/svelte have tests?

It seems not, thank you for checking

I think also @astrojs/solid, @astrojs/preact, and @astrojs/partytown don't have tests

@VoxelMC
Copy link
Contributor

VoxelMC commented Jan 31, 2024

Doing @astrojs/alpinejs now!
I may need some assistance for this, as it uses Playwright instead of Mocha. Either that, or someone else should take it!

@ematipico
Copy link
Member Author

Doing @astrojs/alpinejs now!
I may need some assistance for this, as it uses Playwright instead of Mocha. Either that, or someone else should take it!

If there aren't any mocha tests, we can skip the package

@xMohamd
Copy link
Contributor

xMohamd commented Jan 31, 2024

I will try with @astrojs/lit later today

@VoxelMC
Copy link
Contributor

VoxelMC commented Jan 31, 2024

Doing @astrojs/alpinejs now!
I may need some assistance for this, as it uses Playwright instead of Mocha. Either that, or someone else should take it!

If there aren't any mocha tests, we can skip the package

I will take on @astro/rss instead, then :)

@at-the-vr
Copy link
Contributor

I will take a hit on @astrojs/underscore-redirects

@VoxelMC
Copy link
Contributor

VoxelMC commented Feb 11, 2024

I can take the slots-* tests while I wait for confirmation on my next PR!

@at-the-vr
Copy link
Contributor

I will take collection-collection-* tests

This one is a brainteaser, if someone else could do this that would be super helpful 🙏 (looking at ya Voxel 👀 )

@at-the-vr
Copy link
Contributor

I will take the alias-**.test.js

@ematipico
Copy link
Member Author

I'll take files that:

  • starts with the letter d*
  • starts with the letter e*

@ematipico
Copy link
Member Author

ematipico commented Feb 14, 2024

I'll take files that:

  • starts with v
  • starts with u
  • starts with s
  • starts with t

@at-the-vr
Copy link
Contributor

I will take css-* tests

@ematipico
Copy link
Member Author

I'll take files that start with p* and r*

@ktym4a
Copy link
Contributor

ktym4a commented Feb 15, 2024

I'll take files that start with f*, g* and h*.

I made this files featuresSupport, fetch, fontsource in PR #10115

so I'll take files that start with g* and h*.

@marwan-mohamed12
Copy link
Contributor

I made this files featuresSupport, fetch, fontsource in PR #10115

@ematipico
Copy link
Member Author

I'll take files that start with m*, n* and l*

@ktym4a
Copy link
Contributor

ktym4a commented Feb 15, 2024

JUST MEMO

If my checks are correct, the remaining tests are these:

@ematipico
Copy link
Member Author

I'll i* and j*

@ktym4a
Copy link
Contributor

ktym4a commented Feb 15, 2024

I'll take 0* and c*.

@mingjunlu
Copy link
Contributor

I'll take a*

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P1: chore Doesn't change code behavior (priority) good first issue Good for newcomers. If you need additional guidance, feel free to post in #dev on Discord
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants