mostjs / core

Most.js core event stream
http://mostcore.rtfd.io
MIT License
403 stars 36 forks source link

Discuss a `@most/test` package #230

Open briancavalier opened 6 years ago

briancavalier commented 6 years ago

@nikoskalogridis has been working on a @most/test package. Thank you!

I'd like to use this as a place to discuss the design. Although the core team has discussed it a lot, we've not yet found the time to implement anything. So, I'd like to use this as a place to discuss your design. I think getting on the same page will help us review code more effectively later.

briancavalier commented 6 years ago

I'm primarily interested in the API and ergonomics. @nikoskalogridis could you show some examples of your current direction? Thanks!

nikoskalogridis commented 6 years ago

My initial goal was to expose the helper functions inside the @most/core in order to be usable for testing outside of the @most/core package.

I think its a great idea to discuss the API and ergonomics. It would help if you have a reference of those discussions previously held in order to be up to date as well with the rest of the core members

nikoskalogridis commented 6 years ago

you can have a look at #231 typescript typing file to get an idea of what the api is going to be up to now. Since its the first time that I write typescript please let me know if there are errors. Also helping out with flow (mainly of the way it should be structured) is really appreciated.

Anyway if you think this direction is totally wrong let me know. I was hopping to finish a first beta version during the weekend as I will not have that much time during the next week. But you are right lets discuss this first then put the effort.

briancavalier commented 6 years ago

Great, thanks @nikoskalogridis. No problem and TS and Flow, we can def help.

One overarching thing I'm thinking currently is that since testEnv and all the associated stuff has evolved over quite a long period of time (it started over in cujojs/most), taking a fresh look at what we'd want a simplest-possible test API to look like would be good.

For example, if we have a solid virtual timer, then it's quite simple to construct test streams with at() + merge(), or at() + continueWith(). I could imagine an incredibly simple API foundation being just collectEventsFor! Starting small initially might be a good way to get people using it so we can learn about testing use cases outside of mostjs/core.

Thoughts?

nikoskalogridis commented 6 years ago

I agree that @most/test should be minimal and intuitive.

As I see it, it needs to provide:

  1. Mocked stream creation functions (ie. atTimes, makeEventsFromArray, makeEvents, etc)
  2. an API for running the source stream up specific points in virtual time and collect the result events timestamped (ie. collectEventsFor)
  3. Assertion utility functions for testing the events returned after running the stream (and by those I mean timed event collection assertion functions)

for the first we can expose the atTimes, makeEventsFromArray and makeEvents and for the second part we have collectEventsFor.

so one could write a test as follows:

describe('Adder', () => {
  it('should add 1 on each value of the source stream', () => {
    const source = map(x => x + 1, makeEventsFromArray(1, [1, 2, 3, 4]))
    const expected = [
      {time: 0, value: 2},
      {time: 1, value: 3},
      {time: 2, value: 4},
      {time: 3, value: 5}
    ]
    return collectEventsFor(4, source).then(eq(expected))
  })
})

which is great!

One issue that I run to and puzzled me at first is that if I write:

collectEventsFor(2, source)

the promise is never going to be resolved. As the stream has not ended and in the code we have this:

const collectEvents = curry2(function (stream, scheduler) {
  const into = []
  const s = tap((x) => into.push({time: currentTime(scheduler), value: x}), stream)
  return runEffects(s, scheduler).then(() => into)
})

I think we should make it resolve with the events collected up to dt = 2.

also regarding the mocked stream utilities I think we should also provide an interface for creating mocked stream from an Iterator returning {time: x, value: y} values (do we have a type for this?)

nikoskalogridis commented 6 years ago

one idea for collectEventsFor is to rename it to collectEvents and provide a chained method of building execution. ie

collectEvents
  .from(source)
  .fromTime(4)
  .upToTime(6)
  .then(...)

or something like

collectEvents(
  fromTime(4),
  toTime(6),
  from(source)
).then(...)
nikoskalogridis commented 6 years ago

Regarding the issue with the collectEventsFor not resolving, that was a side effect of fixing VirtualTimer to progress only up to the requested time. And the fix for collectEventsFor would be something like this:

const collectEventsFor = curry2((nticks, stream) =>
  new Promise(function (resolve, reject) {
    const collectedEvents = []
    run(
      {
        end: (time) => resolve(collectedEvents),
        event: (time, value) => time > nticks
          ? resolve(collectedEvents)
          : collectedEvents.push({time, value}),
        error: reject
      },
      ticks(nticks + 1),
      stream
    )
  })
)
nikoskalogridis commented 6 years ago

I would appreciate your help regarding types in flow and more specifically for creating a type for the collectEventsFor.

I have created the following:

export type TimeStampedEvent<A> = {
  time: Time,
  value: A
}

export type TimeStampedEvents<A> = Array<TimeStampedEvent<A>>

export const collectEventsFor =
  curry2(<A>(nticks: Time, stream: Stream<A>): Promise<TimeStampedEvents<A>> =>
    new Promise(function (resolve, reject) {
      const collectedEvents: TimeStampedEvents<A> = []
      run(
        {
          event: function (time, value) {
            if (time > nticks) {
              resolve(collectedEvents)
              return
            }
            collectedEvents.push({time, value})
          },
          end: () => resolve(collectedEvents),
          error: reject
        },
        newScheduler(newVirtualTimer().tick(nticks + 1), newTimeline()),
        stream
      )
    })
  )

the above works but how could I assign the following signatures (which are not accepted by Flow) to this function

interface collectEventsForFn<Time, Stream<A>> {
  (): collectEventsForFn<Time, Stream<A>>,
  (Time): (Stream<A>) => Promise<TimeStampedEvents<A>>,
  (Time, Stream<A>): Promise<TimeStampedEvents<A>>
}
nikoskalogridis commented 6 years ago

I have made some progress with this. Pushed a version using flow for the whole @most/test codebase. If you don't like it I can strip types from the source files.

You can get an overview of the API on the d.ts file. One thing I could not accomplish (with flow or Typescript) is to produce curried versions of the exposed functions yet retaining the signature information of the functions. If anyone could suggest a way please let me know.

Frikki commented 6 years ago

@nikoskalogridis

Arity2 example in TypeScript:

interface CurriedAddFn {
    (x: number, y: number): number
    (x: number): (y: number) => number
}

Perhaps @briancavalier can show how to do it in Flow.

briancavalier commented 6 years ago

@nikoskalogridis Sorry I haven't been able to give any more feedback here yet. I'm splitting my time between several things and trying to keep up. I will make time over the weekend to catch up on the discussion and review #231 so that I can comment.

curried versions of the exposed functions Perhaps @briancavalier can show how to do it in Flow.

Here are the generic curried function types from prelude:

https://github.com/mostjs/core/blob/master/packages/prelude/src/index.js.flow#L37

And here's how we tend to write separate flow defs for curried functions:

https://github.com/mostjs/core/blob/master/packages/core/src/index.js.flow#L9-L10

Do those help, or are you looking for something different?

nikoskalogridis commented 6 years ago

@briancavalier thanks, I understand the way of exposing those functions with the correct signatures at the package level. I was trying to find a way to export those functions curried at the module level and be able to retain their signatures instead of the CurriedFunction2<any, any> between the internal modules.

Anyway I don't think flow and Typescript supports this.

I have updated the code to expose the correct signatures for both Typescript and Flow and expose the functions curried and with the correct signatures on the package level.

joshburgess commented 6 years ago

+1 for a @most/test package. I'm currently trying to test bindings to @most/core I wrote for fp-ts, and I'm finding myself just copying & tweaking code from the test helpers in the src, because they aren't exposed anywhere.

pbadenski commented 5 years ago

Just wondering is this proposal abandoned?

Frikki commented 5 years ago

There seems to be a compelling case for such a package. I wonder what the requirements for such a package would be? Is this still in the air, @joshburgess ?

proProbe commented 5 years ago

I am also interested in this package! has the proposal been dropped?