refactor: run tests with vitest

[bradenmacdonald]

This PR demonstrates changing a few tests to run using Vitest instead of jest. Vitest is largely Jest-compatible and it can also handle all the non-standard imports (of e.g. SVG and SCSS files) we have in this repo.

Running the tests with Vitest is much faster than running them with Jest, though not as fast as using Node's simple built-in test runner.

Time taken to run npm test -- src/Button/Button*.test.tsx:

	Time with coverage	Time (no cov)
node:test	4.76s	3.29s
Vitest	5.48s	4.16s
Jest	41.80s	10.40s

[openedx-webhooks]

Thanks for the pull request, @bradenmacdonald!

This repository is currently maintained by @openedx/committers-paragon.

Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review.

🔘 Get product approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

• If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
  • This process (including the steps you'll need to take) is documented here.
• If it doesn't, simply proceed with the next step.

🔘 Provide context

To help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:

• Dependencies

  This PR must be merged before / after / at the same time as ...

• Blockers

  This PR is waiting for OEP-1234 to be accepted.

• Timeline information

  This PR must be merged by XX date because ...

• Partner information

  This is for a course on edx.org.

• Supporting documentation
• Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

---

Where can I find more information?

If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources:

• Overview of Review Process for Community Contributions
• Pull Request Status Guide
• Making changes to your pull request

When can I expect my changes to be merged?

Our goal is to get community contributions seen and reviewed as efficiently as possible.

However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:

• The size and impact of the changes that it introduces
• The need for product review
• Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

[netlify]

✅ Deploy Preview for paragon-openedx-v23 ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	e7315b7
🔍 Latest deploy log	https://app.netlify.com/projects/paragon-openedx-v23/deploys/6940aa57dea3440008214b3b
😎 Deploy Preview	https://deploy-preview-4043--paragon-openedx-v23.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[holaontiveros]

this one looks awesome!

the node test definitely it's faster but talking more holistically if we had to apply it everywhere and take decisions and handle all the edge cases that node test doesn't manage by default it would be a huge pain.

are we looking to take a decision on which path is better?

[bradenmacdonald]

Yes, because we're relying on a lot of webpack magic that Vite[test] knows how to handle, converting everything to node:test is going to be a pain, even though it's in many ways cleaner and faster. But Vitetest is basically a drop-in replacement for Jest, so I'm proposing that we just switch to Vitest in this and other repos.

Currently there isn't much advantage in installation time because this repo depends on frontend-build which depends on Jest, but if we can separate those out and remove Jest as a depenency altogether it will really decrease the installation size I think.

[bradenmacdonald]

Personally, I think that Vitest makes more sense for any existing repos that have large test suites using Jest, because it's so easy to replace Jest with Vitest. But for new repos or MFEs that are easier to convert, I'd prefer to use node:test which has all the features we need but is faster and has fewer dependencies. I'm fine with just using Vitest everywhere though if that simplicity is preferred.

[bradenmacdonald]

@arbrandes @brian-smith-tcril FYI, you might be interested in these findings.

[arbrandes]

this repo depends on frontend-build

It's just because of the example project, right?

Otherwise, yeah, looks like an easy win! @brian-smith-tcril, should we try and fit this into frontend-base stretch goals for Verawood?

[bradenmacdonald]

It's just because of the example project, right?

Yep.

[brian-smith-tcril]

I think that Vitest makes more sense for any existing repos that have large test suites using Jest, because it's so easy to replace Jest with Vitest. But for new repos or MFEs that are easier to convert, I'd prefer to use node:test which has all the features we need but is faster and has fewer dependencies.

💯

This makes perfect sense to me. node:test would be ideal, but considering how large our existing test suites are in MFEs it'd be a huge effort.

For new (read: frontend-base native) repos, writing the tests with node:test in mind sounds perfect.

---

Currently there isn't much advantage in installation time because this repo depends on frontend-build which depends on Jest

This makes me think making a new breaking release of frontend-build that uses vitest instead of jest is likely a good idea. Thoughts?

[bradenmacdonald]

This makes me think making a new breaking release of frontend-build that uses vitest instead of jest is likely a good idea. Thoughts?

If we're doing a breaking change, I would really like to separate frontend-build and frontend-test, because there's no need to install Vitest/Jest/eslint/etc. when you just want to build an MFE as part of your Tutor/CI workflow.

[brian-smith-tcril]

This makes me think making a new breaking release of frontend-build that uses vitest instead of jest is likely a good idea. Thoughts?

If we're doing a breaking change, I would really like to separate frontend-build and frontend-test, because there's no need to install Vitest/Jest/eslint/etc. when you just want to build an MFE as part of your Tutor/CI workflow.

I'm open to that assuming we could get all MFEs moved to the split versions in time for the Verawood dependency freeze. @arbrandes thoughts?

[brian-smith-tcril]

@bradenmacdonald I am curious as to how you envision the fronted-build/frontend-test split (specifically from an MFE package.json structure perspective)

My main concern is that we maintain a solid dependencies vs devDependencies split. Even without the testing/linting libraries, frontend-build itself should be a devDependency (which is the case in frontend-app-authn but not in frontend-app-authoring) as it is a tool used to build the bundle, not something that should be part of the bundle.

I remember a proposal to move frontend-build from devDependencies to dependencies to facilitate the use of --omit=dev. That breaks the solid split. Instead, I think what we could do is have:

• dependencies
  • Things we need in the bundle (such as frontend-platform)
• devDependencies
  • Things we need to build the bundle (such as frontend-base)
• optionalDependencies
  • Things that are useful for other situations (CI, local dev, etc. such as frontend-test)

Then we could utilize --omit=optional and have maintain the clear split.

[bradenmacdonald]

@brian-smith-tcril I just remember that the dependency situation was way more complicated than I initially thought, and my original proposals weren't actually possible. What you're suggesting now sounds reasonable to me, but I'm not really confident enough to comment on it authoritatively without trying it out.

[arbrandes]

@brian-smith-tcril

assuming we could get all MFEs moved to the split versions in time for the Verawood dependency freeze

Let's take a step back, for a minute. With a move to vitest, the big win is an order-of-magnitude reduction in test run time (big plus for local development), and a significant reduction in total CI time (not an order of magnitude because of install time). The hypothesis is that factoring out a frontend-test would allow us to omit its instalation in certain scenarios. Which scenarios are these? In other words, what user story are we improving?

And just as importantly, what would we lose with a split? I'm thinking of frontend-base, as it would mean a frontend-test that's split out from it as well. As a developer, I've grown fond of having a single repo to deal with for all common frontend code.

In any case, I feel like we should focus any such breaking changes into frontend-base and its MFE conversions, for the simple reason the risk is more tolerable (we're already breaking stuff anyway) and the Verawood scope smaller.

[brian-smith-tcril]

Let's take a step back, for a minute

Agreed. Let's discuss this as 2 separate things.

build/test split

The hypothesis is that factoring out a frontend-test would allow us to omit its instalation in certain scenarios. Which scenarios are these? In other words, what user story are we improving?

when you just want to build an MFE as part of your Tutor/CI workflow.

I'm thinking of frontend-base, as it would mean a frontend-test that's split out from it as well. As a developer, I've grown fond of having a single repo to deal with for all common frontend code.

I don't think this applies in frontend-base.

With frontend-base, there's one site build.

The pain of MFE build times comes into play because of how many MFEs are built as part of tutor-mfe. For example, putting a new footer in a plugin slot requires rebuilding every MFE, which means installing every frontend-build dependency for every MFE even if frontend-build itself is in the MFE's devDependencies.

With frontend-base that same story of replacing the footer would mean 1 rebuild of the site and 1 install of the frontend-base dependencies.

It also sounds like ideally we'd move to using node:test for frontend-base, which would eliminate the dependency issue entirely (but this is definitely a ways off, we don't want to blow the scope of MFE -> app migration by adding reworking every test to the todo list).

Move from jest to vitest

To make this happen, we would need to update frontend-build to include vitest instead of jest (even without a build/test split). This would be a breaking change (even if it's mostly a drop-in replacement).

If you could clarify what you mean by "any such breaking changes" in

I feel like we should focus any such breaking changes into frontend-base and its MFE conversions

that'd be very helpful. Does that mean that you're opposed to making a new breaking release of frontend-build that uses vitest? Or are you specifically referring to a build/test split?

frontend-build options for Verawood

Option 1: Don't change frontend-build.

This would mean we keep jest. build/platform based MFEs would have the same slow tests and would still install jest during tutor-mfe rebuilds.

Option 2: Move frontend-build to vitest. Don't split out test.

This would be a breaking change. It wouldn't improve the "install during tutor-mfe rebuilds" story, but it would improve the CI and local dev stories.

This would mean frontend-build versions 15.x would use vitest, and if we wanted to split out test we could do that for Willow by making 16.x.

Option 3: Split frontend-build into build and test

This would mean frontend-build versions 15.x would not include any testing libraries as dependencies. It would also improve the tutor-mfe rebuild story.

frontend-base considerations for Verawood

If we want to support vitest and node:test (as discussed in #4043 (comment) and #4043 (comment)), we'd need to update the openedx test command.

[bradenmacdonald]

We could ease into things by moving the test dependencies into frontend-test, but making frontend-build declare frontend-test as a dependency for now. But if we're already doing a breaking change with Vitest, it may not be much advantage.

In any case, the big win comes from using Vitest and separating out the test dependencies is not as important to me; more a nice to have.

[brian-smith-tcril]

@bradenmacdonald moving to vitest sounds like a wonderful idea, but this will need to wait until after the Verawood cut.

The main focus for the Verawood release cycle is frontend-base, and any changes to frontend-build, frontend-platform, or MFEs that are being migrated will need to make their way into frontend-base too.

This means making this change in frontend-build would increase the scope of frontend-base work for Verawood, and we already have a lot on our plate there.

We now have openedx/public-engineering#486 to track this effort and develop a plan for landing these improvements in a way that benefits both build/platform based MFEs and frontend-base.

[bradenmacdonald]

Sounds good @brian-smith-tcril. I'm going to close this PR for now since it's incomplete and we have openedx/public-engineering#486 to track it. But it'll remain as a reference for when we decide to take that work on.