False-positive unit tests of observables

[TarasCZ]

Hello, I found that most of your unit tests that tests observable are false-positive, meaning that these tests will pass even when they are supposed to fail.

Minimal reproduction of the bug with instructions:

just change behavior of effect or put one false-positive except (except(false).toBeTruthy()) into the subscribe method and test will still pass

Expected behavior:

Test will fail when they are supposed to

Other information:

example:

   effect.login.subscribe(() => {
     expect(localStorageService.setItem).toHaveBeenCalledWith(AUTH_KEY, {
        isAuthenticated: false
      });
      expect(false).toBeTruthy(); // should obviously fail but it won't
      expect(router.navigate).toHaveBeenCalledWith(['']);
    });
  });

this test has its 'except' in subscribe method, which is asynchronous and will be run after the test will complete without error

I would be willing to submit a PR to fix this issue:

Yes, I would like to start working on PR if you agree.
I would also remove these cold and hot helpers because most of the time here we are testing one value emitted from the observable and there are not necessary.

[tomastrajan]

Hi @TarasCZ !

Thank you very much for finding this, it would be amazing to make this right! I would go with standard @rxjs/testing with TestScheduler which enables simple marble testing without real time!

Docs

Thanks mate!

[TarasCZ]

Hey Tomas :)
I think your solution is bit of an overkill for these simple purposes, since we don't need to test observable behavior over time, also I think is confusing when there is such unnecessary test because ngrx is immutable and shouldn't depend on when the action is fired.
I think the KISS rule could be applied here, I propose use BehaviorSubject and done function like this:

Let me know what do you think about this :)
Thanks, Jirka

[tomastrajan]

@TarasCZ hi Jirka!

Yeah testing of actions is maybe too much ... still having even simple test using TestScheduler can be used as a starting point on how to use it for more complex cases.

Timing of action DOES matter for more complex effects with delays, debounces, race and other complex orchestration which are part of real world applications :)

What do you say ?