Look into groupBy test

[benlesh]

In #1701, there was a test I commented out because its intent wasn't entirely clear, but it was broken after changes to Subject (to match Rx4 semantics) effected the subjects returned by groupBy.

Here is the test:

rxjs/spec/operators/groupBy-spec.ts

Lines 1126 to 1204 in e9445c9

	// HACK: I found this test hard to grok, and it was broken after the Subject refactor.
	// it('should return inners that when subscribed late exhibit hot behavior', () => {
	// const values = {
	// a: ' foo',
	// b: ' FoO ',
	// c: 'baR ',
	// d: 'foO ',
	// e: ' Baz ',
	// f: ' qux ',
	// g: ' bar',
	// h: ' BAR ',
	// i: 'FOO ',
	// j: 'baz ',
	// k: ' bAZ ',
	// l: ' fOo '
	// };
	// const e1 = hot('-1--2--^-a-b-c-d-e-f-g-h-i-j-k-l-|', values);
	// const e1subs = '^ !';
	// const expected = '--v---w---x-y-----z-------|';
	// const subv = ' ^ ';
	// const v = '--------(d|)' ;
	// const subw = ' ^ ';
	// const w = '----------------(h|)' ;
	// const subx = ' ^ ';
	// const x = '----------------------(k|)' ;
	// const suby = ' ^';
	// const y = '------------------------------|';
	// const subz = ' ^';
	// const z = '--------------------------------|';
	// const expectedGroups = {
	// v: Rx.TestScheduler.parseMarbles(v, values),
	// w: Rx.TestScheduler.parseMarbles(w, values),
	// x: Rx.TestScheduler.parseMarbles(x, values),
	// y: Rx.TestScheduler.parseMarbles(y, values),
	// z: Rx.TestScheduler.parseMarbles(z, values)
	// };
	// const subscriptionFrames = {
	// foo: Rx.TestScheduler.parseMarblesAsSubscriptions(subv).subscribedFrame,
	// bar: Rx.TestScheduler.parseMarblesAsSubscriptions(subw).subscribedFrame,
	// baz: Rx.TestScheduler.parseMarblesAsSubscriptions(subx).subscribedFrame,
	// qux: Rx.TestScheduler.parseMarblesAsSubscriptions(suby).subscribedFrame,
	// foo2: Rx.TestScheduler.parseMarblesAsSubscriptions(subz).subscribedFrame
	// };
	// const hasSubscribed = {};
	// const source = e1
	// .groupBy(
	// (val: string) => val.toLowerCase().trim(),
	// (val: string) => val,
	// (group: any) => group.skip(2)
	// )
	// .map((group: any) => {
	// const arr = [];
	// const subscriptionFrame = hasSubscribed[group.key] ?
	// subscriptionFrames[group.key + '2'] :
	// subscriptionFrames[group.key];
	// rxTestScheduler.schedule(() => {
	// group
	// .materialize()
	// .map((notification: Rx.Notification<any>) => {
	// return { frame: rxTestScheduler.frame, notification: notification };
	// })
	// .subscribe((value: any) => {
	// arr.push(value);
	// });
	// hasSubscribed[group.key] = true;
	// }, subscriptionFrame - rxTestScheduler.frame);
	// return arr;
	// });
	// expectObservable(source).toBe(expected, expectedGroups);
	// expectSubscriptions(e1.subscriptions).toBe(e1subs);
	// });

We need to capture it's purpose and drop, refactor or rewrite the test as necessary.

cc/ @staltz

[benlesh]

Closing this issue as stale.

[staltz]

I'm reopening this as I'm now refactoring/rewriting it.

The purpose is: make sure that the inner Observables (GroupedObservables) emitted are hot Observables, backed by a Subject, so late subscribers lose past events.

Why it is broke after the Subject rework: because the group duration subscriber subscribes before the "late subscriber", and the duration subscriber is a TakeOperator, which means once it receives the second next, it completes the GroupedObservable before the late subscriber is able to get that next event.

I'm working on rewriting this test.

[staltz]

Closing because the PR was merged.

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.