-
Notifications
You must be signed in to change notification settings - Fork 61
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
Add testAll function to generate tests from a list of inputs #12
Conversation
external describe : string -> (unit -> unit) -> unit = "describe.only" [@@bs.val] | ||
end | ||
|
||
module Skip : sig | ||
external test : string -> (unit -> 'a A.t) -> unit = "test.skip" [@@bs.val] | ||
external testAsync : string -> (('a A.t -> unit) -> unit) -> unit = "test.skip" [@@bs.val] | ||
external testPromise : string -> (unit -> 'a A.t Js.Promise.t) -> unit = "test.skip" [@@bs.val] | ||
val testAll : string -> 'a list -> ('a -> 'b A.t) -> unit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be external
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed it should
Thanks for doing this - I have updated some of the tests to use It looks pretty good to me, the only slightly strange thing is that the assertions now come before the setup, ie the list of bids and messages is fed into the function which then creates the board in the right state and performs the test. This setup will presumably be done for each assertion whereas previously it was only being done once. I am seeing some new errors when running the other tests, I'm not sure what they are related to, and I have to go to work now...
|
FYI I resolved the error above by changing my Jest configuration from
to
|
Hmm, that's a weird error and solution. Not sure what that's all about, so let's just ignore it and hope it goes away ;) You could just wrap the tests in a |
agree it was a weird error :) I could wrap all the tests in a describe but then it seems like |
I don't see why that'd be odd. |
Fixes #11
Went for a
testAll
function instead ofexpectAll
/expectForEach
/expectMany
. While this restricts the design possibilities a bit, the major benefit is that it will continue to test the remaining inputs if any of the inputs fail, which is definitely worth it IMO.cc @frankwallis, would you like to test this out to see how it fits before I merge?