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.

Sign up for GitHub

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

Jump to bottom

Clarify e2e dependency on yarn in contributin docs #43287

Merged
merged 8 commits into from Dec 9, 2022
Merged

Clarify e2e dependency on yarn in contributin docs #43287

Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
50aab6c
Clarify e2e dependency on yarn in contributin docs
jankaifer Nov 23, 2022
c6b620b
Reformulated sentence in docs due to a linter rule
jankaifer Nov 23, 2022
93b8cbf
Add COREPACK_ENABLE_STRICT=0 before yarn pack
jankaifer Nov 23, 2022
f20bc63
fixed docs
jankaifer Nov 28, 2022
f661636
made yarn pack invocation work in non-bash shells
jankaifer Dec 1, 2022
342cf7a
Merge branch 'canary' into contributing-docs-yarn
jankaifer Dec 1, 2022
7eae25b
Merge branch 'canary' into contributing-docs-yarn
ijjk Dec 8, 2022
e6ae50f
Merge branch 'canary' into contributing-docs-yarn
jankaifer Dec 9, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
15 changes: 15 additions & 0 deletions contributing/core/testing.md
View file
Open in desktop
Expand Up @@ -33,6 +33,21 @@ When the test runs it will open the browser that is in the background by default
pnpm testonly test/integration/production/ -t "should allow etag header support"
```

### Running E2E tests

You will need `yarn` for running e2e tests.
Installing `yarn` with `corepack` is causing issues with our setup so we recommend installing it with `pnpm`:

```sh
pnpm -g install yarn
```

e2e tests are located in `test/e2e`. You can run them like this:

```sh
pnpm testheadless test/e2e/
```
Copy link
Member

Choose a reason for hiding this comment

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

I think we don't need a new section for this, just a small note that e2e tests currently require yarn to be installed globally. No need to mention Corepack.

We could rather mention how e2e tests are run, something like:

End-to-end (e2e) tests are run in complete isolation from the repository. When you run an e2e test, a local version of next will be created inside your system's temp folder (eg. /tmp), which is then linked to the app, also created inside a temp folder. A server is started on a random port, against which the tests will run. After all tests have finished, the server is destroyed and all remaining files are deleted from the temp folder.

@styfle does this sound good?

Copy link
Contributor Author

@jankaifer jankaifer Nov 23, 2022 •
edited

Choose a reason for hiding this comment

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

Argh, my GitHub notifications still don't work.

My main motivation behind this PR is that I spent 2 hours debugging the corepack issue so I wanted to write it down (especially since we are recommending usage of corepack with pnpm installation).
But documenting how e2e works is definitely a good idea 👍

Copy link
Member

Choose a reason for hiding this comment

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

Ideally we would remove the need for yarn and use pnpm everywhere.

The reason there is an issue here is that corepack enforces a single package manager for the entire repo. So it thinks its helping when its actually hurting lol.

There is a new env var we can recommend to turn off this strict check called COREPACK_ENABLE_STRICT=0 that is available since corepack 0.15.0.

I think we should recommend that env var and also look into removing swapping all usages of yarn to pnpm.

Copy link
Contributor Author

@jankaifer jankaifer Nov 23, 2022 •
edited

Choose a reason for hiding this comment

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

I thought that I could add COREPACK_ENABLE_STRICT to the script but it seems that the newest LTS node (v18.12.1) ships with just corepack 0.14.2. So it will not help much for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used @balazsorban44 version and added a short note about corepack.


## Writing tests for Next.js

### Getting Started
Expand Down