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

ngrx9 - setResult fail #2474

Closed
pburkindine opened this issue Apr 4, 2020 · 6 comments
Closed

ngrx9 - setResult fail #2474

pburkindine opened this issue Apr 4, 2020 · 6 comments

Comments

@pburkindine
Copy link

pburkindine commented Apr 4, 2020

ref failed repro attempt:
https://github.com/pburkindine/ngrx9-mockselectors

We are unable to upgrade to ngrx 9 because our ngOnDestroy tests for selector streams in Angular components all fail with:

 Expected one assertion to be called but received zero assertion calls.

      103 |       storeMock.refreshState();
      104 | 
    > 105 |       expect.assertions(1);
          |              ^
      106 |     }));
      107 |   });

Important Note - The test passes when run in isolation.

I think the problem relates to the value already having been set by the previous test and not cleared successfully, as though possibly the storeMock instance were shared or not cleaned up...

...because if I remove the skip(1) in the destroy test it passes, which it shouldn't because the first value should be null. (The only other place that value could have been set onto the stream is the previous "consumes values" test.)

Minimal reproduction of the bug/regression with instructions:

https://github.com/pburkindine/ngrx9-mockselectors

Note that this repro does NOT demonstrate the behavior, but is in every important way identical to the code demonstrating the behavior (a simple string selector in a component, with a consumes and an ngOnDestroy test in the spec, same nx versions & config etc)

Expected behavior:

ngOnDestroy test passes (as in the repro)

Versions of NgRx, Angular, Node, affected browser(s) and operating system(s):

ngrx: 9.0.0
Angular: 9.1
node: 12.13.1
test env: jest 25.2.4

@timdeschryver
Copy link
Member

timdeschryver commented Apr 5, 2020

AFAIK the selectors should reset, and there are also some tests that verify this.

A reproduction here would be useful, as I'm unsure where to start looking.

Have you tried using selectors or function selectors in the real code base, or string selector in the reproduction?

@pburkindine
Copy link
Author

Have you tried using selectors or function selectors in the real code base, or string selector in the reproduction?

I'm not following the question, sorry?

@timdeschryver
Copy link
Member

In the issue you're mentioning that you've used string selectors, but in the reproduction I see that a real selector is being used.

@pburkindine
Copy link
Author

@timdeschryver

In the issue you're mentioning that you've used string selectors, but in the reproduction I see that a real selector is being used.

Sorry, I mean that the selector is selecting a simple string from state (as in, simplest possible case, not an object or anything complicated)

@pburkindine
Copy link
Author

@timdeschryver the code you pointed me to finally got clear what was happening. We had extended MockStore when v8 first came out so that we could implement a fireSelector function (overrideSelector, setState) (see #2000)). The problem was that the code you pointed to was trying to pull the base MockStore from state, finding it missing, and so not resetting the selectors. Since .refreshState is available now, all I needed to do was destroy our extended class. Thanks for pointing me in the right direction, much obliged!

@timdeschryver
Copy link
Member

Awesome! Glad we were able to figure it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants