Skip to content
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 parallel,sequential & hoistSpec #82

Closed
wants to merge 5 commits into from

Conversation

safareli
Copy link
Member

@safareli safareli commented Dec 11, 2018

NOTE: It's still braking change but most of the users will not be affected as only internals are changed and general api is the same

fix #80

the reason is that using generic instance results in:

```
/dev/purescript-spec/output/Data.Generic.Rep.Show/index.js:13
var GenericShowArgs = function (genericShowArgs) {
                               ^

RangeError: Maximum call stack size exceeded
    at new GenericShowArgs (/dev/purescript-spec/output/Data.Generic.Rep.Show/index.js:13:32)
    at Object.genericShowArgsArgument (/dev/purescript-spec/output/Data.Generic.Rep.Show/index.js:20:12)
    at showGroup (/dev/purescript-spec/output/Test.Spec/index.js:241:228)
    at showGroup (/dev/purescript-spec/output/Test.Spec/index.js:241:456)
```
@felixmulder
Copy link
Collaborator

First off, cool 👍 I like that it doesn't break the existing API.

I have some concerns that are purely usability and functional in nature:

What happens when your code crashes? Do we lose all of the logging? Writer just collects right, so if it crashes, I'm assuming you wouldn't get any debug output...

@felixmulder
Copy link
Collaborator

Also, it would be cool to be able to not log unless the test fails :)

@safareli
Copy link
Member Author

What happens when your code crashes? Do we lose all of the logging?

in case of using WriterT over Aff, if Aff crushes due to assertion failure or anything you lose log and ps-spec will mark test as failed.

Writer just collects right, so if it crashes, I'm assuming you wouldn't get any debug output...

yes in case of writer you lose messages, so we should mention that in docs/tests and promote use of Reader instead. nice part about hoistSpec is that users can use anything as long as it can be transformed into aff, so it's less of a responsibility of this lib. tho people want we can provide some defaults type/runner Reader (Ref (Array String)) for example.

@safareli
Copy link
Member Author

Also, it would be cool to be able to not log unless the test fails :)

In some cases you care more about logs when test fails, for diagnostic purposes.

@felixmulder
Copy link
Collaborator

felixmulder commented Dec 11, 2018

Also, it would be cool to be able to not log unless the test fails :)

In some cases you care more about logs when test fails, for diagnostic purposes

My point exactly.

@felixmulder
Copy link
Collaborator

Cool - let me know when you feel this is in a reviewable state!

@safareli
Copy link
Member Author

it's in reviewable state, if API looks good then i would add some more tests/docs

@safareli
Copy link
Member Author

safareli commented Jan 8, 2019

@owickstrom, @felixmulder, I've just added some docs and it's ready for review 🎊

@safareli
Copy link
Member Author

safareli commented Jan 8, 2019

This pr should also fix #68 as users can use Either String Unit for it blocks and in the end use hoistSpec (either throw pure)

@safareli
Copy link
Member Author

safareli commented Jan 9, 2019

I'm working on also adding support for hooks like in hspec, which is not possible without changing the test ADT so soon will open PR with par/sec execution and hooks. most of this code will be changed so no need to review

@safareli safareli closed this Jan 9, 2019
@felixmulder
Copy link
Collaborator

Hi!

Thanks for keeping at this, I've been on vacation, but I'm back now. Looking forward to the next PR :)

@safareli safareli mentioned this pull request Jan 10, 2019
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a way to run tests in parallel
2 participants