Document behaviour of "simultaneous" value syntax about time HOT 11 CLOSED

To line up the diagrams, you could ignore whitespace in them. If you want to emit a whitespace, you can put it inside of parentesis.

from time.

const A =         Time.diagram('--a   -------a---');
const B =         Time.diagram('--b   ---------b-');
const expected$ = Time.diagram('--(ab)-------a-b-');

If you want to emit a space:
const A = Time.diagram('--( )----( )---');

from time.

So we should clearly document:

• A simultaneous value is fired where it starts, at the left parenthesis
• Every character in every diagram represents the passage of time, and this is necessary for tests to line up visually
• Specifically reinforce the behavior as it applies to simultaneous syntax

from time.

Actually, this one also behaves in an unexpected manner. Is it me? 😉

    it("supports multiple simultaneous values", done => {
        const Time = mockTimeSource();

        const stream$ = xs.merge(
            Time.diagram(`--A--B-`),
            Time.diagram(`--0--1-`)
        );

        const expected$ = Time.diagram(`--(A0)--(B1)-`);

        Time.assertEqual(stream$, expected$);
        Time.run(done); // Not OK :(
    });

 FAIL  src/cycle/time.test.js
  ● @cycle/time learning test › supports multiple simultaneous values

    
    Expected
    
    --(A0)-----(B1)
    
    Got
    
    --(A0)--(B1)
    
    Failed because:
    
     * Right value at wrong time, expected at 160 but happened at 100 ("B")
     * Expected value at time 160 but got different value at 100
    
    Diff (actual => expected):
    
     * Right value at wrong time, expected at 160 but happened at 100 (1)
     * Expected value at time 160 but got different value at 100
    
    Diff (actual => expected):
      
      
      
      
          
      at checkEqual (node_modules/@cycle/time/dist/assert-equal.js:59:24)
      at Object.assert.finish (node_modules/@cycle/time/dist/assert-equal.js:72:17)
      at node_modules/@cycle/time/dist/mock-time-source.js:20:62
      at Array.forEach (native)
      at finish (node_modules/@cycle/time/dist/mock-time-source.js:20:20)
      at node_modules/@cycle/time/dist/mock-time-source.js:74:74
      at Immediate.processEvent (node_modules/@cycle/time/dist/run-virtually.js:8:9)
      at runCallback (timers.js:572:20)
      at tryOnImmediate (timers.js:550:5)
      at processImmediate [as _immediateCallback] (timers.js:529:5)

from time.

These both look like bugs. Thanks for reporting 😄

from time.

Sorry, I didn't read this closely enough before, this is actually intended behavior. Consider this example:

  const Time = mockTimeSource();

  const A         = Time.diagram('--a-----a---');
  const B         = Time.diagram('--b-------b-');
  const expected$ = Time.diagram('--(ab)--a-b-');

  const actual$ = xs.merge(A, B);

  Time.assertEqual(actual$, expected$);

  Time.run(done);

This issue is really about whether or not we should consider each character in the simultaneous value syntax as the passage of time.

Currently, simultaneous values are emitted at the time where the left parenthesis starts. In every single case currently, a character in a diagram represents a frame of time.

If we changed this so that simultaneous values only counted as a single frame of time, regardless of how many characters are used to express the simultaneous value, we would have to rewrite the above example as follows:

  const A         = Time.diagram('--a-----a---');
  const B         = Time.diagram('--b-------b-');
  const expected$ = Time.diagram('--(ab)-------a-b-');

  const actual$ = xs.merge(A, B);

  Time.assertEqual(actual$, expected$);

Our expected$ no longer lines up correctly after the simultaneous value.

This design comes from how RxJS implements marble testing.

If there's something I haven't considered here I am open to other design choices.

In the meantime I think the action from this issue is that we should better document how the marble syntax works, specifically this case.

from time.

Thanks for your clear explanation!

So we should clearly document:

• A simultaneous value is fired where it starts, at the left parenthesis
• Every character in every diagram represents the passage of time, and this is necessary for tests to line up visually

Agreed! :)

Specifically reinforce the behavior as it applies to simultaneous syntax

Not sure what you mean by that 🤔

If the diagram "parsing" is currently correct, it follows that the string representation of a stream may not be correct. The following test fails intentionally. The expected and actual streams in the error message seem not to correspond with the actual streams.

it.only('diagram string rep', done => {
    const Time = mockTimeSource();

    const stream$ = xs.merge(Time.periodic(60), Time.periodic(60));
    const expected$ = Time.diagram(`---(00)(11)--`);

    Time.assertEqual(stream$, expected$);
    Time.run(done);
})

Expected

---(00)---(11)

Got

---(00)--(11)--(22)--(33)

Failed because:

 * Length of actual and expected differs
 * Right value at wrong time, expected at 140 but happened at 120 (1)
 * Expected value at time 140 but got different value at 120

Diff (actual => expected):

 * Right value at wrong time, expected at 140 but happened at 120 (1)
 * Expected value at time 140 but got different value at 120

Diff (actual => expected):

 * Expected at index 4 was undefined
 * Expected at index 5 was undefined
 * Expected at index 6 was undefined
 * Expected at index 7 was undefined

from time.

Hi @rjkip,

Good spotting, that is inconsistent. I have fixed up how simultaneous events are displayed in errors, and released v0.7.3 with this improvement. Thanks for pointing that out 👍 😄

See my commit (13221e7) for more detail.

from time.

@jvanbruegge I'm having trouble imagining this, could you give an example, ideally adapted from the one above?

from time.

This has been discussed before, but we need a marble diagram DSL spec that specifies all these cases and would also unify RxJS and this project's DSL. I'm not saying that a spec would be a blocker for things to get done in this project, just saying people eventually need to get together and draft it.

from time.

I have fixed up how simultaneous events are displayed in errors, and released v0.7.3 with this improvement.

Great! This is not the end of these diagram issues, but I think this fixes most unexpected behaviour :)

(...) but we need a marble diagram DSL spec (...)

💯

I'd say my issue specifically has been dealt with for now and we can close this issue imo. Thanks all!

from time.