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

feat(component-store): add a testing library #3646

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

BobobUnicorn
Copy link

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Other... Please describe:

What is the current behavior?

There is no simple unified way that NgRx provides to mock a ComponentStore; this adds a mocking library with a simple API to facilitate writing tests for components that use the ComponentStore.

Closes #2767.

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@netlify
Copy link

netlify bot commented Nov 3, 2022

Deploy Preview for ngrx-io ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 6c3985c
🔍 Latest deploy log https://app.netlify.com/sites/ngrx-io/deploys/636410d49e89c10008357b13
😎 Deploy Preview https://deploy-preview-3646--ngrx-io.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

Copy link

@richieforeman richieforeman left a comment

Choose a reason for hiding this comment

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

Before the core team gets to it, I figured I'd give a review here, based on maybe some initial thoughts.

@@ -1043,5 +1043,8 @@
"schematics": {},
"tags": []
}
},
"cli": {
"analytics": false

Choose a reason for hiding this comment

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

Revert this, they probably don't want that.

@@ -0,0 +1,211 @@
import { Provider, Type } from '@angular/core';
import { TestBed } from '@angular/core/testing';
import 'jasmine';

Choose a reason for hiding this comment

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

Given the hard dep on jasmine here, you'd need to be using Jasmine to use this (which makes sense, given the contribution here).

Does it make sense to put this in

~/modules/component-store/testing/jasmine/xxx/yyy

Imaging that the namespace is..

@ngrx/component-store/testing/jasmine

Copy link
Author

Choose a reason for hiding this comment

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

I wonder if it makes sense to make this generic; the consumer would pass in a parameter that can create a "spy", and a second one that takes a spy and configures it to return a value.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or use a testing framework-agnostic test double library such as Sinon.JS.

@@ -22,6 +22,7 @@
"@ngrx/component-store/schematics-core": [
"./modules/component-store/schematics-core"
],
"@ngrx/component-store/testing": ["./modules/component-store/testing"],
Copy link

Choose a reason for hiding this comment

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

On my earlier note, should we update this here as well?

ngrx/component-store/testing/jasmine

token: Type<MockComponentStore<T>>;
}

interface Constructor<ClassType> {

Choose a reason for hiding this comment

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

Nit/opt: This interface allows you to infer that a constructor for a given type was passed.

  • Should this be "ConstructorOf<T>" ?

I do not feel strongly, but I did feel like perhaps this might have sounded like a built-in, and it might be good to disambiguate. /shruggie

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.

Is it possible to add component-store test example to documentation?
4 participants