feat(jest) update Jest builder to use jest-preset-angular v7 by wesleygrimes · Pull Request #246 · just-jeb/angular-builders

wesleygrimes · 2019-02-19T02:19:35Z

This PR replaces the deprecated setupTestFrameworkScriptFile with the new setupFilesAfterEnv option. I also updated the schema.json with the new option and details per the Jest documentation.

This closes #239 and potentially #193

This is my first PR on your project, so feel free to let me know if things should be done differently, or if I should follow a different naming convention.

…FilesAfterEnv

wesleygrimes · 2019-02-19T13:20:48Z

I see the build failed, but I'm not sure it's related to this code.

Here's the error from Travis:

Failed: "Zone is needed for the async() test helper but could not be found. Please make sure that your environment includes zone.js/dist/zone.js"

just-jeb · 2019-02-19T15:22:22Z

First of all thanks for your contribution! The error is related to your change.
The failing part is the e2e test for this builder. It fails because the version of Jest it uses won't recognize setupFilesAfterEnv option. The fix for this is to change the Jest version in the corresponding example application.
Another thing to consider here is that the builder uses Jest as peerDependency, therefore it should support both options (old and new) depending on the installed version of Jest in host application.
It would be also cool to add an e2e test that actually tests that both user-specified and builder-defined setup files are executed.

wesleygrimes · 2019-02-19T15:31:34Z

First of all thanks for your contribution! The error is related to your change.
The failing part is the e2e test for this builder. It fails because the version of Jest it uses won't recognize setupFilesAfterEnv option. The fix for this is to change the Jest version in the corresponding example application.
Another thing to consider here is that the builder uses Jest as peerDependency, therefore it should support both options (old and new) depending on the installed version of Jest in host application.
It would be also cool to add an e2e test that actually tests that both user-specified and builder-defined setup files are executed.

Thanks, I will take a look at the e2e test.

wesleygrimes · 2019-02-19T18:36:56Z

The node 10 build failed, but looks like due to timeout. Can you confirm? The node 8 build did succeed.

just-jeb · 2019-02-20T15:13:39Z

I restarted the node 10 build. Looks like timeout error indeed. However even if it passes I can't merge this PR yet. As I mentioned, both versions shall be supported and the correct one should be applied depending on host Jest version.
Jest team are deprecating it gradually (showing a deprecation warning), I think we should do the same.

wesleygrimes · 2019-02-27T13:16:47Z

I restarted the node 10 build. Looks like timeout error indeed. However even if it passes I can't merge this PR yet. As I mentioned, both versions shall be supported and the correct one should be applied depending on host Jest version.
Jest team are deprecating it gradually (showing a deprecation warning), I think we should do the same.

I'm not exactly sure how to accomplish checking the installed Jest version, but I will dig in and let you know what I find!

Thanks.

just-jeb · 2019-03-11T22:32:42Z

@wesleygrimes Gave it a second thought. Seems that it's not possible to support both versions of Jest in a single version of the builder, because the builder is tied to a specific version of jest-preset-angular which, in turn, works with a specific version of Jest.
Btw, I've just noticed that in this PR you've changed the version of Jest for the builders project, instead of changing the versions of preset in Jest builder package.
Check out this issue for the proper changes.

wesleygrimes · 2019-03-11T22:53:07Z

Thanks! I will take a look and get this PR updated.

…

-Wes

On Mar 11, 2019, at 6:32 PM, Evgeny Barabanov ***@***.***> wrote: @wesleygrimes Gave it a second thought. Seems that it's not possible to support both versions of Jest in a single version of the builder, because the builder is tied to a specific version of jest-preset-angular which, in turn, works with specific version of Jest. Btw, I've just noticed that in this PR you've changed the version of Jest for the builders project, instead of changing the versions of preset in Jest builder package. Check out this issue for the proper changes. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

wesleygrimes · 2019-03-13T16:04:40Z

@enten @meltedspark

I have incorporated @enten changes from #266, but it is now failing in the mutliple-app and single-app tests. I have to step away for a while, but welcome any feedback and assistance in getting these to pass.

Thanks!

enten · 2019-03-13T21:17:22Z

@wesleygrimes
I tried to investigate why multiple-app and single-app tests failed.

Unfortunately, I don't found how to fix them.

I didn't succeed to compile AppComponent during test because its template html file can't be load. I encountered next error: Failed: "Failed to load app.component.html".

I wondered if zone-patch works well because beforeEach function doesn't seem to succeed. Eventually due to a silent error (about template html which can't be load).

We removed some option about html but to remain compatible with latest jest-preset-angular version.

wesleygrimes · 2019-03-13T21:43:16Z

Yeah same here. Maybe @meltedspark can point us in the right direction?

…

-Wes

On Mar 13, 2019, at 5:17 PM, Steven Enten ***@***.***> wrote: @wesleygrimes I tried to investigate why multiple-app and single-app tests failed. Unfortunately, I don't found how to fix them. I didn't succeed to compile AppComponent during test because its template html file can't be load. I encountered next error: Failed: "Failed to load app.component.html". I wondered if zone-patch works well because beforeEach function doesn't seem to succeed. Eventually due to a silent error (about template html which can't be load). — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

just-jeb · 2019-03-14T08:11:30Z

The projects use lock files (yarn lock), so it's not enough to change the dependency version in package.json. In order to apply it correctly you have to run yarn add [-D] dep@dep-version.
Also, I saw that you removed completely the transform property. Why is that?

wesleygrimes · 2019-03-14T10:30:57Z

Oh duh! That makes sense! Thanks. I will run yarn when I get to my desk this morning and push a new commit. Is there a contributing readme somewhere? If not maybe we can put one together and put down some notes on how to pull down the repo and test, etc.. Thanks! -Wes

…

On Mar 14, 2019, at 4:11 AM, JeB ***@***.***> wrote: The projects use lock files (yarn lock), so it's not enough to change the dependency version in package.json. In order to apply it correctly you have to run yarn add [-D] ***@***.*** — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

just-jeb · 2019-03-14T10:32:59Z

Is there a contributing readme somewhere? If not maybe we can put one together and put down some notes on how to pull down the repo and test, etc..

That's a great idea, definitely need to have one!

wesleygrimes · 2019-03-14T16:42:29Z

The projects use lock files (yarn lock), so it's not enough to change the dependency version in package.json. In order to apply it correctly you have to run yarn add [-D] dep@dep-version.
Also, I saw that you removed completely the transform property. Why is that?

@enten can you speak to why we removed the transform property? Was this a compatibility issue with jest-preset-angular?

enten · 2019-03-14T18:18:05Z

@wesleygrimes @meltedspark
It's important to note that jest-preset-angular/preprocessor.js doesn't exist anymore (since jest-preset-angular v7.0.0-alpha.0).

Previously we override transform to set absolute path to the preprocessor. Now jest-preset-angular uses ts-node directly for ^.+\\.(ts|js|html)$ files.

According to jest-preset-angular v7.0.1 migration guide:

If transform was overridden, remove the entry pointing at preprocessor.js and add "^.+\\.(ts|js|html)$": "ts-jest" to the transform-object.

Initially, I thought that we don't need to override transform property. So we removed the transform property.

Finally, after that @meltedspark reminded me the purpose of Jest builder, I think that we need to override transform property to ensure that a jest custom config doesn't forget to declare "^.+\\.(ts|js|html)$": "ts-jest" again (in case where transform property is overridden).

--- a/packages/jest/src/default-config.resolver.ts
+++ b/packages/jest/src/default-config.resolver.ts
@@ -12,6 +12,9 @@ export class DefaultConfigResolver {
   resolveGlobal(workspaceRoot: Path): any {
     const jestPresetFullPath = join(workspaceRoot, jestPresetRootPath);
     return {...defaultConfig,
+      transform: {
+        '^.+\\.(ts|js|html)$': 'ts-jest'
+      },
       snapshotSerializers: [
         getSystemPath(join(jestPresetFullPath, snapshotSerializer)),
         getSystemPath(join(jestPresetFullPath, htmlCommentSerializer))

--

On another side, I wonder if our issue may is related to the preprocessor. Because it was responsive to fix templateUrl and styleUrls properties before process source with ts-node:

it replace templateUrl property by template and wrap value with require() instruction (which resolves html source as string) ;
it replace styleUrls property by styles with an empty array (which is unecessery for unit tests).

Now ts-node stringifyContentPathRegex is used to export html content and ts-node astTransformers is used to declare an AST Transformer which is responsive to fix templateUrl and styleUrls properties.

Conclusion: we need to investigate to ensure that the ast transformer works as expected.

enten · 2019-03-14T18:35:20Z

@wesleygrimes @meltedspark
My apologies: I forget to adjust global property when its value was overriden (according to the jest-preset-angular migration guide).

Sadly, that doesn't fix issue Failed to load app.component.html. I think that the ast transformer used to fix templateUrl and styleUrls properties in sources isn't called.

Even with add moduleFileExtensions: ['ts', 'html', 'js', 'json'] to jest config: tests still failed.

@wesleygrimes
Can you apply changes below and try to run multiple-apps and single-app tests?

--- a/packages/jest/src/default-config.resolver.ts
+++ b/packages/jest/src/default-config.resolver.ts
@@ -12,6 +12,9 @@ export class DefaultConfigResolver {
   resolveGlobal(workspaceRoot: Path): any {
     const jestPresetFullPath = join(workspaceRoot, jestPresetRootPath);
     return {...defaultConfig,
+      transform: {
+        '^.+\\.(ts|js|html)$': 'ts-jest'
+      },
       snapshotSerializers: [
         getSystemPath(join(jestPresetFullPath, snapshotSerializer)),
         getSystemPath(join(jestPresetFullPath, htmlCommentSerializer))
@@ -23,7 +26,10 @@ export class DefaultConfigResolver {
     return {
       globals: {
         'ts-jest': {
-          tsConfig: getSystemPath(join(projectRoot, tsConfigName))
+          tsConfig: getSystemPath(join(projectRoot, tsConfigName)),
+          stringifyContentPathRegex: '\\.html$',
+          astTransformer: [ require.resolve('jest-preset-angular/InlineHtmlStripStylesTransformer') ]
         }
       },
       testMatch: [

just-jeb · 2019-03-16T09:27:56Z

Hi guys. Thanks for your efforts. I almost forgot, but there actually is a version of builder that incorporates the necessary changes (it existed when the preset was alpha).
It is available in this branch with all the necessary changes. Should have brought it up at the very beginning, but completely forgot about it.
I think it should solve you the mystery.

just-jeb · 2019-03-16T09:32:44Z

Related issue: #182

just-jeb · 2019-03-16T09:36:44Z

I created a pull request for you to highlight the changes that should be made.
Most of it already exists here so it's just adding the missing parts.

enten · 2019-03-17T08:42:18Z

@wesleygrimes @meltedspark
Great! Thanks for fixing the issue sirs.

May we can close this PR (#246) and merge PR #269?

just-jeb · 2019-03-17T10:07:13Z

I'd rather merge this PR and delete the other for a few reasons:

Keep the work you've done
The other PR is created from a branch that wasn't meant to be merged to master but released as an alpha (it also depends on alpha version of the preset).
On the other hand this PR is supposed to be more mature and handle all the stuff like updating the tests dependencies etc.

I created the other PR for a reference to make things in this PR work.
So you guys can make the necessary changes and we'll merge this PR. Or I'll make the changes when I have time.

wesleygrimes · 2019-03-18T17:06:20Z

I haven't had time, but I hope to look at this, this week and get this PR updated. Anyone else is free to jump in and update this as well if you have time. Thanks, Wes

…

On Sun, Mar 17, 2019 at 6:07 AM JeB ***@***.***> wrote: I'd rather merge this PR and delete the other for a few reasons: - Keep the work you've done - The other PR is created from a branch that wasn't meant to be merged to master but released as an alpha (it also depends on alpha version of the preset). On the other hand this PR is supposed to be more mature and handle all the stuff like updating the tests dependencies etc. I created the other PR for a reference to make things in this PR work. So you guys can make the necessary changes and we'll merge this PR. Or I'll make the changes when I have time. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#246 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AATy1IVi4zOEgDCD7R_jHKEqkdKsMbpVks5vXhPRgaJpZM4bCCHo> .

-- Thanks, Wes

wesleygrimes · 2019-03-20T12:25:38Z

OK best I could tell from the PR #269 the only thing needing changed was updating packages\jest\src\jest-config\default-config.ts to add the globals property. Is that correct? If so, I have pushed a new commit to reflect that.

wesleygrimes · 2019-03-20T12:35:53Z

@meltedspark looks like the build passed this time. Want to have a look and see if we can get this merged?

just-jeb · 2019-03-20T14:45:36Z

Looks like we're almost there, the only thing that is missing (according to this comment) is adding

 transform: {
       '^.+\\.(ts|js|html)$': 'ts-jest'
 },

to the config. Since it doesn't involve any path calculation it can be added right to the default config (packages\jest\src\jest-config\default-config.ts).

Also, looks like packages/jest/yarn.lock is not updated (it should be). It shouldn't matter for the final version as yarn.lock is not is not published, but for the local development it is better to be in sync.

wesleygrimes · 2019-03-20T15:30:38Z

Looks like we're almost there, the only thing that is missing (according to this comment) is adding
 transform: {
       '^.+\\.(ts|js|html)$': 'ts-jest'
 },
to the config. Since it doesn't involve any path calculation it can be added right to the default config (packages\jest\src\jest-config\default-config.ts).

Also, looks like packages/jest/yarn.lock is not updated (it should be). It shouldn't matter for the final version as yarn.lock is not is not published, but for the local development it is better to be in sync.

Pushed a commit adding the transform property
Regarding the yarn.lock file, this should have been updated previously when I updated the package.json to jest-preset-angular 7.0.0 ? I normally just use npm so I am not as familiar with updating yarn files. Can you help me with next steps on that?

Thanks,
Wes

just-jeb · 2019-03-20T16:54:11Z

Sure, in order to update "yarn.lock" you have to run "yarn add -D jest-preset-angular" (D for dev).

…

On Wed, Mar 20, 2019, 17:30 Wes Grimes ***@***.***> wrote: Looks like we're almost there, the only thing that is missing (according to this comment <#246 (comment)>) is adding transform: { '^.+\\.(ts|js|html)$': 'ts-jest' }, to the config. Since it doesn't involve any path calculation it can be added right to the default config ( packages\jest\src\jest-config\default-config.ts). Also, looks like packages/jest/yarn.lock is not updated (it should be). It shouldn't matter for the final version as yarn.lock is not is not published, but for the local development it is better to be in sync. - Pushed a commit adding the transform property - Regarding the yarn.lock file, this should have been updated previously when I updated the package.json to jest-preset-angular 7.0.0 ? I normally just use npm so I am not as familiar with updating yarn files. Can you help me with next steps on that? Thanks, Wes — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#246 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AJXjb1hsG5pVK6_rFyubNeskZtNuw8Piks5vYlQegaJpZM4bCCHo> .

wesleygrimes · 2019-03-20T17:40:20Z

Sure, in order to update "yarn.lock" you have to run "yarn add -D jest-preset-angular" (D for dev).
…
On Wed, Mar 20, 2019, 17:30 Wes Grimes @.***> wrote: Looks like we're almost there, the only thing that is missing (according to this comment <#246 (comment)>) is adding transform: { '^.+\.(ts|js|html)$': 'ts-jest' }, to the config. Since it doesn't involve any path calculation it can be added right to the default config ( packages\jest\src\jest-config\default-config.ts). Also, looks like packages/jest/yarn.lock is not updated (it should be). It shouldn't matter for the final version as yarn.lock is not is not published, but for the local development it is better to be in sync. - Pushed a commit adding the transform property - Regarding the yarn.lock file, this should have been updated previously when I updated the package.json to jest-preset-angular 7.0.0 ? I normally just use npm so I am not as familiar with updating yarn files. Can you help me with next steps on that? Thanks, Wes — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#246 (comment)>, or mute the thread https://github.com/notifications/unsubscribe-auth/AJXjb1hsG5pVK6_rFyubNeskZtNuw8Piks5vYlQegaJpZM4bCCHo .

Ok, all set, it did update jest-preset-angular to 7.0.1. Hopefully that's ok.

just-jeb · 2019-11-15T12:36:18Z

@all-contributors please add @wesleygrimes for code

allcontributors · 2019-11-15T12:36:26Z

@just-jeb

I've put up a pull request to add @wesleygrimes! 🎉

feat(jest) replace deprecated setupTestFrameworkScriptFile with setup…

c5dc9e0

…FilesAfterEnv

updated jest versions

b45fe78

just-jeb mentioned this pull request Mar 11, 2019

Jest builder with jest-preset-angular v7 #266

Closed

wesleygrimes added 6 commits March 13, 2019 10:44

Merged upstream master

625d7c2

merge upstream master

4bb9cfb

revert changes to main package.json

609d29e

revert example app versions

79444c8

update versions in jest package.json

af91073

incorporate changes from #266

8c90a34

wesleygrimes changed the title ~~feat(jest) replace deprecated setupTestFrameworkScriptFile~~ feat(jest) update Jest builder to use jest-preset-angular v7 Mar 13, 2019

update example app jest deps for tests

21702cf

Update yarn.lock files for ci

a0be252

incorporate changes from pr #269

e7cd231

Adding transform property to default-config

b37c980

Update yarn.lock

eb19e00

Evgeny Barabanov added 4 commits March 21, 2019 10:41

chore: remove transform (already in default jest-preset-angular config)

a32de0c

v7.4.0

191da91

chore: update readme

5af57fe

chore: update examples dependencies

ec6b6f7

just-jeb merged commit 15d62aa into just-jeb:master Mar 21, 2019

allcontributors Bot mentioned this pull request Nov 15, 2019

docs: add wesleygrimes as a contributor #618

Merged

Uh oh!

Conversation

wesleygrimes commented Feb 19, 2019

Uh oh!

wesleygrimes commented Feb 19, 2019

Uh oh!

just-jeb commented Feb 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wesleygrimes commented Feb 19, 2019

Uh oh!

wesleygrimes commented Feb 19, 2019

Uh oh!

just-jeb commented Feb 20, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wesleygrimes commented Feb 27, 2019

Uh oh!

just-jeb commented Mar 11, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wesleygrimes commented Mar 11, 2019 via email

Uh oh!

wesleygrimes commented Mar 13, 2019

Uh oh!

enten commented Mar 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wesleygrimes commented Mar 13, 2019 via email

Uh oh!

just-jeb commented Mar 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

wesleygrimes commented Mar 14, 2019 via email

Uh oh!

just-jeb commented Mar 14, 2019

Uh oh!

wesleygrimes commented Mar 14, 2019

Uh oh!

enten commented Mar 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

enten commented Mar 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

just-jeb commented Mar 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

just-jeb commented Mar 16, 2019

Uh oh!

just-jeb commented Mar 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

enten commented Mar 17, 2019

Uh oh!

just-jeb commented Mar 17, 2019

Uh oh!

wesleygrimes commented Mar 18, 2019 via email

Uh oh!

wesleygrimes commented Mar 20, 2019

Uh oh!

wesleygrimes commented Mar 20, 2019

Uh oh!

just-jeb commented Mar 20, 2019

Uh oh!

wesleygrimes commented Mar 20, 2019

Uh oh!

just-jeb commented Mar 20, 2019 via email

Uh oh!

wesleygrimes commented Mar 20, 2019

Uh oh!

just-jeb commented Nov 15, 2019

Uh oh!

allcontributors Bot commented Nov 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

just-jeb commented Feb 19, 2019 •

edited

Loading

just-jeb commented Feb 20, 2019 •

edited

Loading

just-jeb commented Mar 11, 2019 •

edited

Loading

enten commented Mar 13, 2019 •

edited

Loading

just-jeb commented Mar 14, 2019 •

edited

Loading

enten commented Mar 14, 2019 •

edited

Loading

enten commented Mar 14, 2019 •

edited

Loading

just-jeb commented Mar 16, 2019 •

edited

Loading

just-jeb commented Mar 16, 2019 •

edited

Loading