-
Notifications
You must be signed in to change notification settings - Fork 36
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Discuss a @most/test
package
#230
Comments
I'm primarily interested in the API and ergonomics. @nikoskalogridis could you show some examples of your current direction? Thanks! |
My initial goal was to expose the helper functions inside the 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 |
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. |
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? |
I agree that As I see it, it needs to provide:
for the first we can expose the 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 |
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(...) |
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
)
})
) |
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>>
} |
I have made some progress with this. Pushed a version using flow for the whole 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. |
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. |
@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.
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? |
@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 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. |
+1 for a |
Just wondering is this proposal abandoned? |
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 ? |
I am also interested in this package! has the proposal been dropped? |
@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.
The text was updated successfully, but these errors were encountered: