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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bye-bye, Jest #19886

Merged
merged 18 commits into from
Oct 4, 2023
Merged

Bye-bye, Jest #19886

merged 18 commits into from
Oct 4, 2023

Conversation

paescuj
Copy link
Member

@paescuj paescuj commented Oct 2, 2023

So Long, and Thanks for All the Fish

Scope

What's changed:

  • Switch Blackbox Tests from Jest to Vitest
    • Migrate package from CJS to ESM
    • Remove copy of utils in favor of import from @directus/utils (wasn't possible due to ESM)
    • Update dependencies (some needed to be kept back due to pure ESM)
    • TS issues clean-up (some appeared after updating tsconfig.json, others already there)

Potential Risks / Drawbacks

  • Lot of file changes, albeit about the same in every file
  • Vitest runs slightly faster, increasing pressure during tests 馃槄
  • Vitest runs in greater isolation, requiring a temp file to expose data from sequencer to setup/tests - with Jest data could be passed (more easily) via env
  • In contrast to Jest, Vitest calls the sequencer only after global setup, which means the test flow setup had to be moved from global setup to the common test

Review Questions

  • See above ^^
  • Additionally:
    • Opted to use MSSQL setup on host instead of Docker container to overcome performance issues which became noticeble with migration to Vitest
    • Currently a placeholder package is required for custom environment, see Failed to load custom environment from JS file聽vitest-dev/vitest#4237
    • There are still some leftover type issues from previous setup, but I suggest to tackle this separately
      (see new pnpm typecheck command)

Successful test run: https://github.com/directus/directus/actions/runs/6405405868

@changeset-bot

This comment was marked as off-topic.

@paescuj paescuj requested a review from licitdev October 2, 2023 23:40
@rijkvanzanten

This comment was marked as resolved.

@paescuj paescuj added the Chore label Oct 4, 2023
@paescuj paescuj marked this pull request as ready for review October 4, 2023 11:44
This reverts commit d6fbdd7.
Copy link
Member

@rijkvanzanten rijkvanzanten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The refactoring generally LGTM. There's too much to realistically go over everything in full detail, but seeing we didn't alter the tests contents and the test still pass, this should be good to go 馃憤馃徎

Copy link
Member

@licitdev licitdev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Hello Vitest! 馃憢 @paescuj Thank you for the PR~ 馃挴

On a side note, there are some TS errors in tests but those could be fixed in a separate PR. 馃殌

@rijkvanzanten rijkvanzanten merged commit 1ec1f98 into main Oct 4, 2023
7 checks passed
@rijkvanzanten rijkvanzanten deleted the blackbox-馃殌 branch October 4, 2023 18:53
@github-actions github-actions bot added this to the Next Release milestone Oct 4, 2023
br-rafaelbarros pushed a commit to personal-forks/directus-source that referenced this pull request Nov 7, 2023
* Bye-bye, Jest

* Clean-up non-null assertions for PORT

* Test all vendors

* Consistent file names

* Try with MSSQL 2022

* Revert "Try with MSSQL 2022"

This reverts commit da807c6.

* Try to run on Windows for better MSSQL perf

* Worth a try (hopefully runs MSSQL faster this way)

* Start docker services via bash

* Quick attempt with local installation of MSSQL

* Test with MSSQL 2019

* Back to ubuntu-latest for other tests

* Add typecheck cmd

* Revert "Test all vendors"

This reverts commit d6fbdd7.

* Use TS for Vitest config file

* Simplify vendors typecasting

---------

Co-authored-by: ian <licitdev@gmail.com>
@paescuj paescuj mentioned this pull request Dec 10, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants