feat(mock): support for Jest mocking #40

dirkluijk · 2018-08-19T23:05:32Z

This PR adds support for Jest mocks. It adds an additional entry point @netbasal/spectator/jest to import symbols from. When you do, it will automatically use jest for mocking.

In this PR:

Added new createSpyObject() and mockProvider implementation which uses Jest
Secondary entry point (implemented with ng-packagr), exporting different versions of:
- initialModule() (because it is part of the public API)
- SpectatorWithHost and createHostComponentFactory()
- SpectatorHTTP and createHTTPFactory()
- Spectator and createTestComponentFactory()
- SpectatorService and createService()
Example Jest setup in the demo app
Example Jest test in the demo app (auth-service.jest.ts)
ng-packagr improvements
Typescript overloading improvements

TODO?

Testing in other projects
peerDependencies? (currently, we don't have them)
Add docs page about Jest support

Closes #39.

src/lib/package.json

tonivj5 · 2018-10-09T17:22:58Z

Any progress in this PR?

dirkluijk · 2018-11-16T00:25:56Z

Updated PR description.

Is master the correct target branch?

NetanelBasal · 2018-11-16T06:05:29Z

Wow. Looks great!!

I saw that you added types for our custom matchers to jest? Does jest support the syntax of jasmine matchers?

NetanelBasal · 2018-11-16T06:05:58Z

@benelliott, please help with the review.

dirkluijk · 2018-11-16T10:07:39Z

I think that in the future, we might even move away from default mocking and have it more like this:

import { createSpectator } from '@netbasal/spectator'; // no mocks at all
import { createSpectator } from '@netbasal/spectator/jest'; // with Jest mocks
import { createSpectator } from '@netbasal/spectator/jasmine'; // with Jasmine spies

benelliott · 2018-11-16T10:30:55Z

Looks really cool! Will find some time to review soon.

jpzwarte · 2018-11-16T10:49:08Z

@dirkluijk wow, nice work!

magikMaker · 2018-11-16T11:02:06Z

Great work @dirkluijk 👌

dirkluijk · 2018-11-16T14:25:13Z

Credits to @LayZeeDK for the entry point idea.

dirkluijk · 2018-11-16T15:05:36Z

Added additional tests and improved the typings a little bit.

@NetanelBasal

I saw that you added types for our custom matchers to jest? Does jest support the syntax of jasmine matchers?

Yes, they are compatible. And work perfectly fine (proven in Jest tests in the demo app).

benelliott

Overall it looks great. My only problem with it is that it tidies up a lot of unrelated stuff (various let vs consts, internal imports etc) which IMO clutter the PR a bit and make it harder to see what is actually being changed. I would prefer to see this stuff fixed in a separate PR.

I will mark it as approved and leave it up to @NetanelBasal to decide whether that needs addressing before it can be merged.

In general though - thanks for the hard work!

benelliott · 2018-11-16T17:06:11Z

projects/spectator/jest/src/mock.ts

+ return newSpy;
+ }
+
+ function installProtoMethods(proto: any) {


Code reuse could potentially be improved here by (internally) exporting this function in the base library. Wouldn't want it to be part of the public API though.

benelliott · 2018-11-16T17:06:48Z

projects/spectator/jest/package.json

@@ -0,0 +1,3 @@
+{
+ "ngPackage": {}


Is this just a mandatory file that exposes this part of the project as a submodule?

Exactly. See https:/ng-packagr/ng-packagr/blob/master/docs/secondary-entrypoints.md.

benelliott · 2018-11-16T17:16:15Z

projects/spectator/package.json

@@ -26,11 +25,5 @@
 "dependencies": {
 "dom-testing-library": "2.1.0",
 "jquery": "^3.3.1"


Any particular reason for moving these in this PR? Was something broken?

I generally agree that they should be tidied up but don't think it needs to be included here unless necessary.

In combination with the ng-package.json (to support an additional entry point) I had to move this configuration to the ng-package.json.

benelliott · 2018-11-16T17:20:58Z

projects/spectator/src/lib/matchers-types.d.ts

+
+ toHaveStyle(style: { [styleKey: string]: any }): boolean;
+
+ toHaveData({ data, val }: { data: string; val: string }): boolean;


Narrowing these jasmine matcher types could potentially be a breaking change (?). I see why you have done it though.

You can use overload and not introduce a breaking change

benelliott · 2018-11-16T17:24:30Z

projects/spectator/tsconfig.lib.json

@@ -12,7 +12,7 @@
 "experimentalDecorators": true,
 "importHelpers": true,
 "types": [
- "@types/jasmine"


Kind of surprised this doesn't break anything! Are jest types a superset of jasmine's or something?

@dirkluijk in the past it works for me only because I had both jest and jasmine installed. We should test it on a clean project.

I cannot load both @types/jasmine and @types/jest at the same time. That is because @types/jest redefines Jasmine namespace and typings. See https:/DefinitelyTyped/DefinitelyTyped/blob/master/types/jest/v16/index.d.ts#L178.

NetanelBasal · 2018-11-17T16:08:04Z

projects/spectator/jest/src/mock.ts

+ return newSpy;
+ }
+
+ function installProtoMethods(proto: any) {


NetanelBasal · 2018-11-17T16:09:34Z

projects/spectator/src/lib/is-type.ts

@@ -0,0 +1,5 @@
+import { Type } from '@angular/core';
+
+export function isType(v: any): v is Type<any> {


isFunction?

I actually copied this type guard from Angular: https:/angular/angular/blob/4c2ce4e8ba4c5ac5ce8754d67bc6603eaad4564a/packages/core/src/type.ts

Not sure what is better. Let me know if you insist of isFunction. "Is function" is what the implementation does, but it's intention is to check whether you pass a Type (Component). Also, it is a type guard for Type.

Ok, let's leave it.

NetanelBasal · 2018-11-17T16:10:19Z

projects/spectator/src/lib/matchers-types.d.ts

+
+ toHaveStyle(style: { [styleKey: string]: any }): boolean;
+
+ toHaveData({ data, val }: { data: string; val: string }): boolean;


You can use overload and not introduce a breaking change

NetanelBasal · 2018-11-17T16:11:34Z

projects/spectator/tsconfig.lib.json

@@ -12,7 +12,7 @@
 "experimentalDecorators": true,
 "importHelpers": true,
 "types": [
- "@types/jasmine"


@dirkluijk in the past it works for me only because I had both jest and jasmine installed. We should test it on a clean project.

NetanelBasal · 2018-11-17T16:15:47Z

@dirkluijk After you finish with the fixes will publish a beta version to npm so we can start testing it with the following cases:

Jasmine - clean project.
Jest - clean project.
An existing project should be able to install the new version without any change.

I'll also give you grant to our gitbook so you can update the docs about jest.

dirkluijk · 2018-11-19T10:24:40Z

I will check the review comments and fix them asap.

Please check my replies.

Overall it looks great. My only problem with it is that it tidies up a lot of unrelated stuff (various let vs consts, internal imports etc) which IMO clutter the PR a bit and make it harder to see what is actually being changed. I would prefer to see this stuff fixed in a separate PR.

If you want, I can move these changes to another PR.

I'll also give you grant to our gitbook so you can update the docs about jest.

Awesome.

NetanelBasal · 2018-11-19T12:36:46Z

@dirkluijk let me know when you're done, please. (also pull from master)

dirkluijk · 2018-11-20T20:04:59Z

@NetanelBasal I fixed some issues and pulled from master.
After a new review it might be good to squash all commits in this PR.

Please let me know how I can be of any help.

NetanelBasal · 2018-11-20T20:36:44Z

@dirkluijk will release a new beta version first thing in the morning, and we start to test it. Thanks.

NetanelBasal · 2018-11-21T06:31:25Z

Published v2.3.0-rc.1, you can start with the tests.

dirkluijk · 2018-11-21T15:05:42Z

Awesome. Will test it in a blank project tonight or tomorrow. I will share the repo and its Circle CI build status when I've finished it.

NetanelBasal · 2018-11-27T20:13:24Z

@dirkluijk testing it in our jasmine application and everything is working as usual. I'm waiting for your side about jest.

dirkluijk · 2018-11-28T10:55:56Z

@NetanelBasal thanks. Have been very busy last days, but I will try to make time for it later this week.

NetanelBasal · 2018-12-12T08:34:59Z

@dirkluijk ping

LayZeeDK · 2018-12-16T00:01:58Z

I ran a fresh project with Spectator 2.3.0-rc.1 through Jest with a trivial component and a trivial service with no type errors or run-time errors.

NetanelBasal · 2018-12-16T05:30:20Z

Cool, thanks @LayZeeDK. I don't want to wait anymore with this PR. I will also run a check and publish it.

LayZeeDK · 2018-12-16T12:14:05Z

We could add Jest versions of the remaining specs in this repository to get the full Jest API exercised.

dirkluijk · 2018-12-19T20:11:51Z

Sorry for this late reply. Always hard to leave some spare time for open-source commitment. ;)

For now, I assume everything is working correctly I guess. If not, let me know.

LayZeeDK · 2018-12-19T20:23:10Z

Thanks, Dirk! I use Jest on all my personal projects so I'm very grateful 😊 (Even though I use Sinon for test doubles and Chai for assertions)

dirkluijk mentioned this pull request Aug 19, 2018

Option to use Jest for Spectator mocking #39

Closed

NetanelBasal reviewed Aug 20, 2018

View reviewed changes

src/lib/package.json Outdated Show resolved Hide resolved

dirkluijk changed the base branch from next to master November 16, 2018 00:23

feat(mock): support for Jest mocking

45f0677

NetanelBasal requested a review from benelliott November 16, 2018 05:56

dirkluijk added 2 commits November 16, 2018 16:01

refactor: type improvements

2e56d9e

test: add additional Jest specs

98ea17b

fix(jest): add missing export

a1a210b

benelliott approved these changes Nov 16, 2018

View reviewed changes

NetanelBasal requested changes Nov 17, 2018

View reviewed changes

dirkluijk added 3 commits November 19, 2018 17:14

Merge branch 'master' into spectator-with-jest-and-jest-mocking

84d3265

fix(jest): fixes after review comments

2b7d2a5

fix(jest): fix build

7f058b3

NetanelBasal approved these changes Nov 20, 2018

View reviewed changes

NetanelBasal merged commit 7f058b3 into ngneat:master Dec 16, 2018

dirkluijk deleted the spectator-with-jest-and-jest-mocking branch December 19, 2018 20:11

LayZeeDK mentioned this pull request Dec 23, 2018

Could it work with Jest? #11

Closed


		toHaveStyle(style: { [styleKey: string]: any }): boolean;

		toHaveData({ data, val }: { data: string; val: string }): boolean;

		@@ -0,0 +1,5 @@
		import { Type } from '@angular/core';

		export function isType(v: any): v is Type<any> {

feat(mock): support for Jest mocking #40

feat(mock): support for Jest mocking #40

Conversation

dirkluijk commented Aug 19, 2018 • edited Loading

tonivj5 commented Oct 9, 2018

dirkluijk commented Nov 16, 2018 • edited Loading

NetanelBasal commented Nov 16, 2018

NetanelBasal commented Nov 16, 2018

dirkluijk commented Nov 16, 2018

benelliott commented Nov 16, 2018

jpzwarte commented Nov 16, 2018

magikMaker commented Nov 16, 2018

dirkluijk commented Nov 16, 2018

dirkluijk commented Nov 16, 2018 • edited Loading

benelliott left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NetanelBasal commented Nov 17, 2018 • edited Loading

dirkluijk commented Nov 19, 2018

NetanelBasal commented Nov 19, 2018

dirkluijk commented Nov 20, 2018

NetanelBasal commented Nov 20, 2018

NetanelBasal commented Nov 21, 2018

dirkluijk commented Nov 21, 2018

NetanelBasal commented Nov 27, 2018

dirkluijk commented Nov 28, 2018

NetanelBasal commented Dec 12, 2018

LayZeeDK commented Dec 16, 2018 • edited Loading

NetanelBasal commented Dec 16, 2018

LayZeeDK commented Dec 16, 2018

dirkluijk commented Dec 19, 2018

LayZeeDK commented Dec 19, 2018 • edited Loading

dirkluijk commented Aug 19, 2018 •

edited

Loading

dirkluijk commented Nov 16, 2018 •

edited

Loading

dirkluijk commented Nov 16, 2018 •

edited

Loading

NetanelBasal commented Nov 17, 2018 •

edited

Loading

LayZeeDK commented Dec 16, 2018 •

edited

Loading

LayZeeDK commented Dec 19, 2018 •

edited

Loading