Skip to content

Moved secret service to be observable based

David Burke requested to merge refactor-secrets-effects-test into staging

This could be part of 1.0 release or not. Shouldn't cause any issues. Desc from rocket chat:

Simple usage of - in marble test

    actions = hot("a", { a: new GetSecretsAction() });
    const response = cold("-a", { a: secrets });
    secretService.getSecrets.and.returnValue(response);
    const expected = cold("-b", { b: new SetSecretsAction(secrets) });
    expect(effects.getSecrets$).toBeObservable(expected);

so the change here is that the response happens on a delay - which actually mimics reality better this case is a little trivial - but you could imagine some race condition where timing does matter. next up - testing for a handled error!

    actions = hot("a", { a: new GetSecretsAction() });
    const response = cold("-#", {});
    secretService.getSecrets.and.returnValue( response );
    const expected = cold("-(b|)", { b: new HandleAPIErrorAction("error") });
    expect(effects.getSecrets$).toBeObservable(expected);

The -# means pause 10 frames (with - is 10) and throw a very generic error the -(b|) is a tough one. () just means happens in the same frame. So the b and | happen both at frame 10. | means "complete". So the expected result is that on time frame 10 we get a Handle API Error and the completion of the observable You could also return the error with secretService.getSecrets.and.returnValue( Observable.throw("ah!")); which would replace the response variable. I'm forcing myself to use marbles though to understand it. My understanding of the | syntax is still shaky. I don't actually understand why the success tests don't need it. in fact I kind of suspect it's a strange artifact of the test itself....because why would the effect ever complete? It should just run forever waiting for more dispatched actions

Merge request reports