build(dev-infra): setup basic e2e structure #410
Conversation
ahnpnl
changed the title
|
Sure I will add some explanations for each script |
|
added |
|
|
||
| What will happen in order is as following: | ||
|
|
||
| 1. A bundle (we'll call it `[bundle]`) will be created for `jest-preset-angular` using `npm pack` (`yarn pack` is buggy). |
There was a problem hiding this comment.
I'd suggest running from the source, to simplify setup. But running bundle has its own benefits, so I won't oppose too much
| ## Running tests | ||
|
|
||
| You run the tests on example apps with `yarn test:e2e` (or `npm run test:e2e)`. In `scripts/lib/paths.js`, | ||
| `projectsToRun` must be modified if a new project is added to `e2e`. |
There was a problem hiding this comment.
could be determined automatically using glob package on e2e/ directory. However, I'd much prefer using Jest's "projects" to facilitate e2e testing, which would get rid of custom sync tests running script
There was a problem hiding this comment.
will do in a separate PR to setup yarn workspace and use jest "projects"
scripts/e2e.js
Outdated
|
|
||
| // then we can run the tests | ||
| const useYarn = existsSync(join(monorepoRealPath, 'yarn.lock')) | ||
| const cmdLine = projectPkg.scripts && projectPkg.scripts.test ? [useYarn ? 'yarn' : 'npm', 'test'] : ['jest'] |
There was a problem hiding this comment.
we run "npm" install to then run "yarn test"? would be nicer to pick one package manager (I suggest yarn) and stick to it. I'm fine with using "npm pack" for packing
There was a problem hiding this comment.
ya the part of installing other dependencies can stick with yarn. However, the part of installing the .tgz needs to stick with npm because yarn doesn't have the option to say "no change in package.json when installing this dependency", see yarnpkg/yarn#1743
|
|
||
| ## Why | ||
|
|
||
| E2E tests being in a temporary folder of the operating system ensure none of the parents would be containing `node_modules`. |
There was a problem hiding this comment.
I don't see any reference to creating a temporary dir in the code. I can only see chdir to e2e/<project> and running commands there. In such case, I'd highly suggest using Jest's "projects" feature:
https://jestjs.io/docs/en/configuration#projects-arraystring--projectconfig and use Yarn workspaces to minimize the network payload when downloading all dependencies of the project and e2e tests. This may be challenging to set up, but faster builds, less maintenance and less network payloads are really nice to have
There was a problem hiding this comment.
If there's no Yarn workspace in the project now, I'd consider adding it later, after this PR is merged, because now with e2e it makes sense to use it
There was a problem hiding this comment.
ok let's do it in a separate PR to setup yarn workspace + jest "project"
There was a problem hiding this comment.
The issue is that we have different behavior of the preset for a Angular v9 and v10 App, so we want to test both. If we use yarn workspaces and only install the dependencies once, we should not be able to install different Angular versions. Correct me if I'm wrong.
There was a problem hiding this comment.
Ok actually I think this will work with yarn workspaces, but we will see in the follow-up PR.
There was a problem hiding this comment.
Thumbs up from my side once the comments are resolved 👍
Just to keep track of what to do later:
- Turn project into Yarn Workspace
- Use
globto determine which projects are located ine2einstead of modifyingpaths.jseach time - Move
/exampletoe2eastest-app-v9 - add
test-app-v10to CI
|
made a push for:
The rest of the comments are for another PR which is related to yarn workspace and jest |
Summary
Added
scriptsfolder which includese2e.jsto run e2e tests with example apps. This folder also contains some helpers.Added an example app using Angular 10 with basic setup for
jestandjest-preset-angular.Added new scripts to
package.json.Test plan
yarn test:e2ein your local.Additional information
This PR only creates a basic e2e structure setup but won't include this into CI yet. The next PR will do it (also including moving the app in
exampletoe2efolder)