README feedback about time HOT 6 CLOSED

I found all questions answered. Nothing to complain about.

from time.

I think it should be mentioned somewhere that completion times are not taken into account when comparing streams. An example: '-x-|' == '-x---|'.

from time.

@rjkip I might change that. My original rationale was that sometimes you want to make assertions against diagrams where the last event and completion occur in the same frame (character in diagram).

Like:

it("creates a stream that emits every period ms", (done) => {
  const Time = mockTimeSource();

  const stream = Time.periodic(40);

  const expected = Time.diagram(
    `----0---1---2|`
  );

  Time.assertEqual(
    stream.take(3),
    expected
  );

  Time.run(done);
});

The expected diagram states that completion occurs the frame after the last event, whereas in reality it occurs immediately. So there is lenience in asserting and a kludge in diagram generation to support this case.

The real fix for this issue is so support syntax for multiple items in a single frame, which I plan on investigating soon.

Even then I think it would be ugly to have to do:

it("creates a stream that emits every period ms", (done) => {
  const Time = mockTimeSource();

  const stream = Time.periodic(40);

  const expected = Time.diagram(
    `----0---1---(2|)`
  );

  Time.assertEqual(
    stream.take(3),
    expected
  );

  Time.run(done);
});

I'm also considering another kludge where I would check the time difference between completion events for actual and expected and fail if it's more than one frame different.

from time.

@rjkip I have released v0.6 which will fail a test if the actual and expected streams end at different times.

As part of this I have also added syntax to express simultaneous events, as per the above example.

However, to improve the readability of the above test, I have allowed for testing infinite streams without completion:

it("creates a stream that emits every period ms", (done) => {
  const Time = mockTimeSource();

  const stream = Time.periodic(80);

  const expected = Time.diagram(
    `----0---1---2---`
  );

  Time.assertEqual(
    stream,
    expected
  );

  Time.run(done);
});

Even though periodic is infinite, now Time.run() will only run to the end of your longest diagram (if you've used a diagram). So we can now test infinite streams without using .take, which is nice, and they will no longer run forever if you forget to use .take.

from time.

@rjkip I have released v0.6 which will fail a test if the actual and expected streams end at different times.

That sounds good. Confirmed to work as well :)

It does make a test that tests for a value and a completion in the same frame a bit awkward, but I think that's acceptable. These things are hard. You can still change it in a future minor, though :)

Time.assertEqual(Time.periodic(20).take(1), Time.diagram(`-(0|)`));

[...] I have allowed for testing infinite streams without completion:

Great! I was wondering about options there :)

All in all, thanks for replying and creating and maintaining this library 👌

from time.

It does make a test that tests for a value and a completion in the same frame a bit awkward, but I think that's acceptable. These things are hard.

Agreed, but I figure for a testing tool it's important to prioritize accuracy and correctness. I would rather be explicit and slightly awkward than implicit and imprecise.

In any case, this will only really come up when you have a stream designed to emit in the last frame, like of and last. It's thankfully not the common case for Cycle.js apps.

from time.