Skip to content

Test suite #51

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

Merged
merged 107 commits into from
Jan 17, 2025
Merged

Test suite #51

merged 107 commits into from
Jan 17, 2025

Conversation

coopbri
Copy link
Contributor

@coopbri coopbri commented Dec 31, 2024

Description

Task link: https://linear.app/omnidev/issue/OMNI-87/integrate-playwright-msw-for-e2e-testing, https://linear.app/omnidev/issue/OMNI-151/set-up-unit-test-framework-bun-vanilla-if-possible

Note

This PR is not intended to provide robust coverage over our existing codebase, rather to get the foundation started and merged in so we can agree on a clean config and then build out tests (including more fixtures, mocks, etc.) as a team later. Further, there are lots of TODOs introduced in this PR, but they are all incredibly related, mostly via pending app router support.

Warning

MSW does not currently fully support the Next.js app router, so E2E tests are skipped for now as a consequence. Track mswjs/examples#101. Once this is merged, it can be used as an integration reference, and the tests can be enabled.

Test Steps

Before testing the tests by testing them in a testful fashion, make sure to set the new environment variables (both adding the new ones and setting up .env.test.local).

  • Offer feedback on configuration for Playwright, Bun test runner + Happy DOM, and the test CI workflow (I am open to thoughts on changing anything, lots of new config here)
    • One specific question: is uploading E2E test artifacts useful in the first place? We could simplify the workflow and speed up execution time by skipping it. In the future, we will likely have a more robust CI integration with artifact uploads. This question is connected to the disabled merge_e2e_reports job + corresponding TODO Decision: scrap
    • Another question: instead of .env.test.local, it might be desirable to just load the local .env.local if running E2E tests locally. Let me know thoughts on this! Had some annoyances getting it working but it might be nice, I'm not sure about long-term benefits leaning into the decision for either approach Decision: consolidate
    • Make sure to offer feedback on file naming and directory structure! Open to thoughts on naming convention changes and anything else
  • Test local unit tests: bun run test (note that some are skipped, see colocated code for details)
  • Test local E2E tests: bun test:e2e (note that these tests are skipped, see warning above; just make sure the command works)

coopbri and others added 30 commits December 31, 2024 16:15
"plugins": [
{
"name": "next"
}
],
"baseUrl": "src"
"types": ["bun"]
Copy link
Contributor

Choose a reason for hiding this comment

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

Ohhh does this work for all DefinitelyTyped packages? If so that is cool. I have always added @types/bun, etc here haha

Copy link
Contributor Author

@coopbri coopbri Jan 10, 2025

Choose a reason for hiding this comment

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

Honestly I'm not sure if it applies to all cases of DT (or bundled/non-DT) packages, but nice lil shortcut for several that I've tried

Copy link
Contributor

@benjamin-parks benjamin-parks left a comment

Choose a reason for hiding this comment

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

Tests working the same as shown in screenshots.

@coopbri coopbri merged commit 558cc20 into master Jan 17, 2025
2 checks passed
@coopbri coopbri deleted the test/suite branch January 17, 2025 00:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants