Marble testing for when inner observables unsubscribe

[benlesh]

This is related to discussion in #423 ...

Basically the problem is that in something like switch, we don't have a way of asserting when inner subscriptions were unsubscribed from.

[benlesh]

one thought I had was reusing the ! in the expectation marbles, and then matching it up to an array of unsusbscription assertions some how:

let a = hot('--1--^--------2--------3-----');
let b = hot('--4--^-----5----------------6------7---');
let observableLookup = { a: a, b: b };
let e1 = hot('----^------a-------b-------', observableLookup);
let expected = '---------^-2-----(!^)----6------7---';

expectObservable(e1.switch()).toBe(expected).withSubs([a, b]).withUnsubs([a]);

.... it's not a particularly good idea, but it's a start.

I don't really like that it junks up the expected marbles, though.

[benlesh]

Perhaps the inner subs and unsubs could be in different marble diagrams.

The worst part though will be doing this without adding test specific code to the library. We may have to create custom TestObservable and TestSubject types that deal with this by creating a TestSubscriber instead of a Subscriber when subscribe() is called.

[staltz]

To mirror RxJS 4, the equivalent of this type of call https://github.com/Reactive-Extensions/RxJS/blob/master/tests/observable/selectmany.js#L240-L244 in our marble diagram tests would be

let a = hot('--1--^--------2--------3-----');
let asubs = '------------^-------!';
let b = hot('--4--^-----5----------------6------7---');
let bsubs = '--------------------^------------------';
let observableLookup = { a: a, b: b };
let e1 = hot('----^------a-------b-------', observableLookup);
let expected =   '---------2-------------6------7---';

expectObservable(e1.switch()).toBe(expected);
expect(a.subscriptions).toBe(asubs);
expect(b.subscriptions).toBe(bsubs);

I'm not sure though when should the first - of asubs start, but roughly that's the idea. I think it would work. And maybe we should make expectSubscriptions instead of overriding expect. Although it would be pretty cool if a.subscriptions would be precisely the marble diagram string to be matched with the asubs string. We just need to replicate the ideas in RxJS 4. Such as creating a ColdObservable. I guess this is comparable to the TestObservable and TestSubject you just mentioned. But I can't see another way.

[benlesh]

The thing about it, is we know that all observables there have a subscription point. And that, in part, that subscription point can be inferred by the marble tests we already have in existance. Subscription starts when the marble line starts, in essence.

So really, I think we only need to assert unsubscriptions when (and if) they happen.

[staltz]

Let me get it right, you're suggesting this?:

let a = hot('--1--^--------2--------3-----');
let aunsubs =           '--------!';
let b = hot('--4--^-----5----------------6------7---');
let bunsubs =                   '------------------';
let e1 = hot('----^------a-------b-------', { a, b });
let expected =   '---------2-------------6------7---';

expectObservable(e1.switch()).toBe(expected);
expect(a.unsubscriptions).toBe(aunsubs);
expect(b.unsubscriptions).toBe(bunsubs);

---

I bumped into a case where I want to have multiple subscribers to a shared Observable, so I need expectObservable to have a parameter indicating when to subscribe. I'm trying to make tests for share, shareReplay, shareBehavior, etc.

Ideally, I'd like to write this:

var e1 = cold(   '--1--2------3--------4---5---6--7--|');

var asubs =   '---^---------------------------------!';
var aexpects =   '--1--2------3--------4---5---6--7-'

var bsubs =   '---------^-----------------!';
var bexpects =         '(12)--3--------4--'

var csubs =   '------------------^---------------------!';
var cexpects =                  '(123)-4---5---6--7--|'

var shared = e1.shareReplay(3);

expectObservable(shared, asubs).toBe(aexpects);
expectObservable(shared, bsubs).toBe(bexpects);
expectObservable(shared, csubs).toBe(cexpects);

This proposed API wouldn't be an obstacle for the previous suggestion I gave. In fact I think they could be used together, for instance:

var e1 = cold(   '--1--2------3--------4---5---6--7--|');

var asubs =   '---^---------------------------------!';
var aexpects =   '--1--2------3--------4---5---6--7-'

var bsubs =   '---------^-----------------!';
var bexpects =         '(12)--3--------4--'

var csubs =   '------------------^---------------------!';
var cexpects =                  '(123)-4---5---6--7--|'

var shared = e1.shareReplay(3);

expectObservable(shared, asubs).toBe(aexpects);
expectObservable(shared, bsubs).toBe(bexpects);
expectObservable(shared, csubs).toBe(cexpects);
expect(shared.subscriptions).toBe([asubs, bsubs, csubs]); // <--- this

[benlesh]

I like it, but that will only tell the test when to subscribe and unsubscribe. We still don't have a way to assert when inner observables unsubscribe or subscribe. But I think we can use the same diagrams.

Think of testing switch, for example. You need to ensure that the inner observables are subscribed to and unsubscribed from at the proper time.

So we can take the same subscription diagrames say something like: expectSubscription(observableReference, subscriptionMarbles), or in the case above, expectSubscription(e1, asubs). (if you wanted to assert that the subscription occurred, rather than make it occur).

[staltz]

Are we roughly in agreement about the overall approach? I can try tackling this issue and also making some tests with any helper I build. (Although I would start work on this only next Monday because here it's already Friday evening.)

[benlesh]

I think so. No worries, on the delay. I'm unsure if you've looked into it, but basically there are instance methods on TestScheduler and I add helpers to a file under specs/helpers.

[staltz]

Working on this now. Essentially building ColdObservable, HotObservable, SubscriptionLog classes and modifying TestScheduler and specs/helpers.

[staltz]

@Blesh I identified a case where it would make sense to assert when the inner observables subscribe as well as unsubscribe.

  it('should mergeMap many outer values to many inner values', function () {
    var values = {i: 'foo', j: 'bar', k: 'baz', l: 'qux'};
    var e1 =    hot('-a-------b-------c-------d-------|');
    var inner = cold('----i---j---k---l---|', values);
    var expected =  '-----i---j---(ki)(lj)(ki)(lj)(ki)(lj)k---l---|';

    expectObservable(e1.mergeMap(function(value) { return inner; }))
      .toBe(expected, values);
  });

Could be adapted to assert also:

  it('should mergeMap many outer values to many inner values', function () {
    var values = {i: 'foo', j: 'bar', k: 'baz', l: 'qux'};
    var e1 =    hot('-a-------b-------c-------d-------|');
    var inner = cold('----i---j---k---l---|', values);
    var innersubs =['-^-------------------!',
                    '---------^-------------------!',
                    '-----------------^-------------------!',
                    '-------------------------^-------------------!']
    var expected =  '-----i---j---(ki)(lj)(ki)(lj)(ki)(lj)k---l---|';

    expectObservable(e1.mergeMap(function(value) { return inner; }))
      .toBe(expected, values);
    expectSubscriptions(inner.subscriptions).toBe(innersubs);
  });

Roger?

[staltz]

Should now be done with PR #463.

[staltz]

This issue can be closed.