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

Unsubscribe from marble tests #402

Closed
ktrott opened this issue Sep 24, 2015 · 6 comments
Closed

Unsubscribe from marble tests #402

ktrott opened this issue Sep 24, 2015 · 6 comments
Assignees
Milestone

Comments

@ktrott
Copy link

ktrott commented Sep 24, 2015

Can't do tests with the marble tests to show if someone wants to unsubscribe at some point: explicit disposable vs. completion. Need some way to tell the test scheduler to unsubscribe from the test observable at a particular frame.

@staltz
Copy link
Member

staltz commented Sep 29, 2015

While making tests for mergeMap I think I figured one potential way of making this happen.


The subscription is done in TestScheduler.expect():

    this.schedule(() => {
      observable.subscribe((value) => {
        actual.push({ frame: this.frame, notification: Notification.createNext(value) });
      }, (err) => {
        actual.push({ frame: this.frame, notification: Notification.createError(err) });
      }, () => {
        actual.push({ frame: this.frame, notification: Notification.createComplete() });
      });
    }, 0);

We need to keep that subscription inside expect(). We can change expect's signature to be

expect(observable: Observable<any>, unsubscriptionMarbles?: string)

So you can optionally provide a marble diagram for the unsubscription moment. And then use it like this:

var e1 = hot('--a-^---b--c----d-------e--f--g--|');
var expected =    '-----b--c----d---';
var unsub =  '----------------------|';

expectObservable(e1.delay(20), unsub).toBe(expected);

Notice the second parameter given to expectObservable.

What do you think? Does this solve also tricky use cases we had for switch/switchMap? I would know how to implement that above.

@benlesh
Copy link
Member

benlesh commented Sep 29, 2015

That's not bad. But wouldn't the unsub marbles be:

var e1 = hot('--a-^---b--c----d-------e--f--g--|');
var expected =   '-----b--c----d---';
var unsub =      '----------------------|';

expectObservable(e1.delay(20), unsub).toBe(expected);

So that the ^ and first -'s are aligned?

@staltz
Copy link
Member

staltz commented Sep 29, 2015

Correct, was my mistake.

@benlesh
Copy link
Member

benlesh commented Sep 29, 2015

@staltz, I like it, but I think the character should be different than |... maybe this is a good place for !, since it looks similar. That would also mean fewer changes and assertions in the code to parse the marbles and process them.

@staltz
Copy link
Member

staltz commented Sep 30, 2015

Ok, let's do exclamation then. Another character I considered was underscore, because it would look a bit like the opposite of ^. Subscribe is up ^, unsubscribe is down _.

var e1 = hot('--a-^---b--c----d-------e--f--g--|');
var expected =   '-----b--c----d---';
var unsub =      '----------------------_';

expectObservable(e1.delay(20), unsub).toBe(expected);

But I'm fine with exclamation:

var e1 = hot('--a-^---b--c----d-------e--f--g--|');
var expected =   '-----b--c----d---';
var unsub =      '----------------------!';

expectObservable(e1.delay(20), unsub).toBe(expected);

@staltz
Copy link
Member

staltz commented Oct 7, 2015

This issue can be closed since it's fully implemented.

@benlesh benlesh closed this as completed Oct 7, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Jun 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants