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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(unit): update unit tests in the example apps #1165

Closed
wants to merge 5 commits into from
Closed

test(unit): update unit tests in the example apps #1165

wants to merge 5 commits into from

Conversation

rfprod
Copy link
Contributor

@rfprod rfprod commented Nov 11, 2021

Summary

  • add more unit tests to existing example applications as requested in #110;
  • add include property to tsconfigs for esm specs; without it esm specs are failing, can't find exports;

Test plan

All unit tests should be green.
I tested locally everything (including yarn test-esm) except Angular v13 tests.
I could not make Angular v13 unit tests work locally, but I am pretty sure the specs that were added should pass.

Does this PR introduce a breaking change?

  • No

Other information

@rfprod
Copy link
Contributor Author

rfprod commented Nov 11, 2021

Hm, ESM tests are failing. I have not tested these locally.
I wonder why exporting interface does not work with ESM support.

...
PASS src/app/services/foo.service.spec.ts

Test Suites: 3 passed, 3 total
Tests:       8 passed, 8 total
Snapshots:   0 total
Time:        4.64 s
Ran all test suites.
starting the ESM tests using: yarn test-esm --no-cache

$ yarn workspace angular-app test-esm --no-cache
$ node --experimental-vm-modules ../../node_modules/jest/bin/jest.js -c=jest-esm.config.js --no-cache
ngcc-jest-processor: running ngcc
(node:3113) ExperimentalWarning: VM Modules is an experimental feature. This feature could change at any time
FAIL src/app/app.component.spec.ts
  ● Test suite failed to run

    SyntaxError: The requested module '../interfaces/environment.interface' does not provide an export named 'IAppEnvironment'
...

PS: Oh, I see..

@rfprod
Copy link
Contributor Author

rfprod commented Nov 11, 2021

I'll push a fix in a minute.

@ahnpnl
Copy link
Collaborator

ahnpnl commented Nov 15, 2021

Hi, thanks a lot for your effort. Actually the purpose of #110 is to include all the tests of the Angular example app Tour of Heroes. It would be better if you can convert those tests there to use Jest and add them to the example apps in examples folder.

@rfprod
Copy link
Contributor Author

rfprod commented Nov 15, 2021

@ahnpnl I'll take a look.
So far I added test cases that helped discover the missing part in esm tsconfigs. Take a look please 607ce7e#diff-cb4c71ab5c17e1a2099d6c08a788541b59a59a45ee3b8cdbd79133bb89b10728

If it is removed esm tests will fail.

@ahnpnl
Copy link
Collaborator

ahnpnl commented Nov 15, 2021

@ahnpnl I'll take a look. So far I added test cases that helped discover the missing part in esm tsconfigs. Take a look please 607ce7e#diff-cb4c71ab5c17e1a2099d6c08a788541b59a59a45ee3b8cdbd79133bb89b10728

If it is removed esm tests will fail.

It is better to do "include": ["src/environments"] instead of "include": ["src/**/*.ts"]. In this case, because environments folder is not a part of compiler therefore the error is thrown.

Another thing I just noticed is: test ESM with isolatedModules: true failed (after rebase).

@rfprod rfprod marked this pull request as draft November 15, 2021 17:05
@rfprod
Copy link
Contributor Author

rfprod commented Nov 16, 2021

@ahnpnl I rebased, updated tsconfigs, and updated unit tests so that it works.
One thing that I'm trying to figure out right now is failing esm tests in Angualr v13 example app.
Both commands fail with the same error:

  • yarn test-esm
  • yarn test-esm-isolated

All tests in other example apps passed successfully locally.

jest-preset-angular/examples/example-app-v13$ yarn test-esm
yarn run v1.22.17
$ node --experimental-vm-modules --no-warnings node_modules/jest/bin/jest.js -c=jest-esm.config.js --no-cache
ngcc-jest-processor: running ngcc
 FAIL  src/app/guards/route.guard.spec.ts
  ● Test suite failed to run

    Must use import to load ES Module: /media/suser/DATA/git/rfprod/jest-preset-angular/examples/example-app-v13/node_modules/@angular/core/fesm2015/testing.mjs

      at Runtime.requireModule (node_modules/jest-runtime/build/index.js:972:21)

 FAIL  src/app/app.component.spec.ts
  ● Test suite failed to run

    Must use import to load ES Module: /media/suser/DATA/git/rfprod/jest-preset-angular/examples/example-app-v13/node_modules/@angular/core/fesm2015/testing.mjs

      at Runtime.requireModule (node_modules/jest-runtime/build/index.js:972:21)

 FAIL  src/app/services/foo.service.spec.ts
  ● Test suite failed to run

    Must use import to load ES Module: /media/suser/DATA/git/rfprod/jest-preset-angular/examples/example-app-v13/node_modules/@angular/core/fesm2015/testing.mjs

      at Runtime.requireModule (node_modules/jest-runtime/build/index.js:972:21)

Test Suites: 3 failed, 3 total
Tests:       0 total
Snapshots:   0 total
Time:        2.232 s
Ran all test suites.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

- [X] add more specs to existing example application;
- [X] add include property to tsconfigs for esm specs;
- [X] rebase onto the main branch;
 - [X] include src/environment in compilation instead of
src/**;- [X] fix unit tests after rebasing the branch;

test(unit): fix leftovers

- [X] fix unit tests in example apps v11, 12, 12-monorepo, yarn workspace;
@ahnpnl
Copy link
Collaborator

ahnpnl commented Nov 16, 2021

ESM mode with Angular 13 isn’t supported yet :)

- [X] refactor app component specs;
- [X] fix typos in environment config notes;
@rfprod
Copy link
Contributor Author

rfprod commented Nov 16, 2021

@ahnpnl got it. I reviewed my changes again, made several improvements and fixes.
Do you want me to add the Tour of Heroes sample app sources and tests within this PR or as another PR?
And just for clarification, you want me to add sources and tests from here, right?

@ahnpnl
Copy link
Collaborator

ahnpnl commented Nov 17, 2021

I suggest an approach like following:

  • Make this PR to add unit tests from Tour of Heroes app (the link you linked is correct) for ONLY one example app, for example we do it for example-app-v11 in this PR.

  • Make several other separate PRs, each of them for one example app. For example, next PR will be for example-app-v12, another one for example-app-v13. Doing in separate PRs make it easier to review too.

  • We can skip example-app-v10 since Angular v10 will be end of life on Dec 24, 2021

@rfprod
Copy link
Contributor Author

rfprod commented Nov 18, 2021

@ahnpnl Closing this PR. I opened new drafts. Will update them soon.
#1185
#1182
#1183
#1181
#1184
#1185

@rfprod rfprod closed this Nov 18, 2021
@rfprod rfprod deleted the unit-test branch November 26, 2021 20:30
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.

2 participants