Repository: mostjs/core
Pull request #285 (2019-05-19 16:06:32)

Fix duplicate addition of offset time on switchLatest
This fixes https://github.com/mostjs/core/issues/284.

I confirmed that the original `switchLatest` emits an error event with time `2`, which should be `1`, and removing ` + this.min` from `Segment#_dispose` resolves the issue.

To be honest, I find my way of writing test case pretty clumsy, but I could not find a  straightforward way to examine the case.
Feel free to request changes, I'll appreciate it.

Thank you.

briancavalier: Hey @gitusp, thanks for doing this!

This is very much an edge case.  That said, I agree it's probably harder than it should be to observe the time value in this particular case.

Here's a thought on a slightly simpler approach using some of the testing utilities in core/test/helpers.  I'm sorry these are as well-documented as we'd like them to be.  FakeDisposeStream is from test/helpers/FakeDisposeStream.

```js
// new imports
import FakeDisposeStream from './helper/FakeDisposeStream'
import { run } from '../src/run'

 // ...
      const inner = FakeDisposeStream.from(() => { throw new Error() }, at(1))
      const s = at(1, inner)
      const sink = {
        event () {},
        end (t) {},
        error (t, e) {
          try {
            eq(2, t)
            done()
          } catch (e) {
            done(e)
          }
        }
      }

      run(sink, ticks(2), switchLatest(s))
```

What do you think?
gitusp: Thank you @briancavalier! That's way simpler and cleaner.

I applied your suggestion and pushed again.
Sorry for having you write almost all the code. 🙇
briancavalier: > Sorry for having you write almost all the code.

Haha, no worries at all, @gitusp.  Team work!

Patch:
diff --git a/packages/core/src/combinator/switch.js b/packages/core/src/combinator/switch.js
index aea3787..58d37fd 100644
--- a/packages/core/src/combinator/switch.js
+++ b/packages/core/src/combinator/switch.js
@@ -110,6 +110,6 @@ class Segment {
   }
 
   _dispose (t) {
-    tryDispose(t + this.min, this.disposable, this.sink)
+    tryDispose(t, this.disposable, this.sink)
   }
 }


Test patch:
diff --git a/packages/core/test/switch-test.js b/packages/core/test/switch-test.js
index 1338fd4..8c2f396 100644
--- a/packages/core/test/switch-test.js
+++ b/packages/core/test/switch-test.js
@@ -6,12 +6,15 @@ import { take } from '../src/combinator/slice'
 import { constant, map, tap } from '../src/combinator/transform'
 import { periodic } from '../src/source/periodic'
 import { empty, isCanonicalEmpty } from '../src/source/empty'
+import { at } from '../src/source/at'
 import { now } from '../src/source/now'
 import { ticks, collectEventsFor, makeEvents, makeEventsFromArray } from './helper/testEnv'
+import FakeDisposeStream from './helper/FakeDisposeStream'
 import { runEffects } from '../src/runEffects'
+import { run } from '../src/run'
 
 describe('switch', () => {
-  it('given c anonical empty string, should return canonical empty', () => {
+  it('given canonical empty string, should return canonical empty', () => {
     const s = switchLatest(empty())
     assert(isCanonicalEmpty(s))
   })
@@ -86,4 +89,27 @@ describe('switch', () => {
         ]))
     })
   })
+
+  describe('when upper stream fails to dispose', () => {
+    it('should emit an error event with correct time', (done) => {
+      // See https://github.com/mostjs/core/issues/284
+
+      const inner = FakeDisposeStream.from(() => { throw new Error() }, at(1))
+      const s = at(1, inner)
+      const sink = {
+        event () {},
+        end (t) {},
+        error (t, e) {
+          try {
+            eq(2, t)
+            done()
+          } catch (e) {
+            done(e)
+          }
+        }
+      }
+
+      run(sink, ticks(2), switchLatest(s))
+    })
+  })
 })