add seq/par and hook #83

safareli · 2019-01-10T13:52:28Z

fix #80
fix #43
fix #44 (any m can be lifted into Spec so you can lift Effect too)
fix #63
fix #68 (in the end user can hoist Either to Aff)

it might somewhat fixes #73 haven't looked into it yet

Initially I tired to rebase #52 on top of #82 but using the approach taken in #52 it was not possible to define afterAll and nesting of hooks was not possible too. So instead I ported this version from hspec.

things left to do:

need to fix reporter to handle case when there is nesting of describe in parallel (see logs of delaySpecExample)
add docs of hooks (probably copy and modify from hspec)

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) ```

safareli · 2019-01-10T14:17:03Z

This is ready for review @felixmulder @owickstrom. tomorrow will try to fix issue with reporter (about parallel nested describes) and add some docs. let me know if there are some comments/suggestions.

felixmulder · 2019-01-10T17:39:34Z

Cool! Won't have time to review today - but hopefully during the weekend 🙂

as they are not Aff specific any more

now consoleReporter and specReporter need to coordinate tests which are running in parallel

safareli · 2019-01-11T09:06:46Z

The issue I have now is that consoleReporter and specReporter need to handle case like this:

describe "a" $ parallel do
  it "a.1" $ delay $ Milliseconds 500.0
  it "a.2" $ delay $ Milliseconds 500.0
  describe "b" do
    it "b.1" $ delay $ Milliseconds 500.0
    it "b.2" $ delay $ Milliseconds 500.0

so a reporter will get events in this order, which is messing output:

Suite a
Test a.1
Test a.2
Suite a.b
Test a.b.1
Test a.b.2
Pass a.1 + TestEnd a.1
Pass a.2 + TestEnd a.2
Pass a.b.1 + TestEnd a.b.1
Pass a.b.2 + TestEnd a.b.2
SuiteEnd a.b
SuiteEnd a

solution is to make reporters more "advanced", by showing currently running tests with some spinner, and then once they finish update corresponding indicators, ideally so that if a test is logging something to console, it's not interfering to the test output (I haven't done such things in terminal and some suggestions/helpful links, would be great help for fixing this issue)

safareli · 2019-01-11T11:16:35Z

AFAICT without some kind of mocking/monkey patching, a reporter has no way to know if a running test logged something to terminal. If that's true, we have 2 options:

ignore case when some test logs to console and be ok that it will mess output.
fancy reporters like consoleReporter and specReporter will take Bus (Path, String) as argument and they will be watching for messages and safely printing them so output is not messed up.

I would at first go with option 1 and then probably in new PR implement option 2 so, before that using console.log in parallel tests will mess output at bit.

safareli · 2019-01-15T08:39:28Z

so i was able to update spec/console reporters to handle parallel execution:

But, :(

if a line printed to console is wrapped, then redraw is messed up and output looks like this:

this particular issue can be fixed in countLines function, if reporter subscribes to width of terminal (with process.stdout.getWindowSize and process.stdout.on("resize")) but I think it might not work for all terminals&OS combinations.

so at this point I feel that the "redrawing" terminal output is not a good idea, so will try to simplify reporters and will log things to console which are not going to change, it would mean that if you are running multiple tests in parallel you wouldn't see there output until whole all of them are finished.

only log things to console when execution of test is finished so we don't need to do redraw

felixmulder · 2019-01-15T19:35:39Z

Wow, awesome work @safareli !

You've really put time into this.

felixmulder · 2019-01-15T19:44:25Z

Quite a large PR - but I couldn't see anything that raised a red flag with me. @owickstrom, maybe you'd like to review as well?

safareli · 2019-01-16T07:02:46Z

You've really put time into this.

Yeah, was working on it for about a week, we are using this lib at @f-o-a-m, and we wanted to have this features, so it was sort of my "task".

with `exit: false` and you can get test results with `[]` as reporters you can get no reporting so removed runSpec,runSpec' and renamed run,run' to runSpec and runSpecM

for example here all types are derived by compiler: ``` foo :: forall t19. Monad t19 => SpecT Aff Unit t19 Unit foo = it "foo" $ delay $ Milliseconds 100.0 baz :: forall t17 t18 t19. Monad t19 => SpecT t18 t17 t19 Unit baz = describe "baz" $ pure unit fix :: forall t18 t19. Monad t19 => MonadThrow Error t18 => SpecT t18 Unit t19 Unit fix = it "fiz" $ 1 `shouldEqual` 1 ``` vs beofre you would get huge `WriterT ...` type.

safareli · 2019-01-16T12:23:58Z

It would be great if someone could check changes made in documentation.

owickstrom · 2019-01-26T09:19:33Z

I'll have a look when I'm back from vacation. Thanks for your effort on this @safareli!

owickstrom

A lot of cool stuff going on, I hope I didn't miss anything. Mostly my comment about compatibility that I would stress, and perhaps the test.

Otherwise, really awesome work! 🎉

owickstrom · 2019-02-01T18:59:31Z

src/Test/Spec/Runner.purs


 -- | Run the spec with the default config
-run
+runSpec


I think runSpec is a better name, but can we for compatibility reasons also have run? As an alias of runSpec, perhaps with a deprecation Warn constraint?

owickstrom · 2019-02-01T19:03:28Z

test/Test/Spec/RunnerSpec.purs

        it "supports async" do
          res <- delay (Milliseconds 10.0) *> pure 1
          res `shouldEqual` 1
+  where
+    runSpecFocused t = discardUnfocused (execWriter $ un SpecT t) <#> bimap (const unit) (un Item >>> _.isFocused)


runSpecFocused, if I'm not misunderstanding, isn't running the full runSpec machinery, like this test did before. It seems it's collecting a tree structure of Maybe Bool values for isFocused. Would it be possible to rewrite this to use runSpec (or some underlying function) to exercise the runner module?

Most I could do is to add collect let me know if there is something you think is worth adding/changing.

safareli · 2019-02-07T13:56:00Z

Besides what was commented as part of the review I have change type of hoistSpec and mapSpecTree so the type in which SpecTrees are accumulated can also be changed, it can be useful for example if some part of test is using Spec type (which uses Identity) and then you want to use it in some SpecT with other monad like this: hoistSpec (un Identity >>> pure) (const identity) someTest

safareli · 2019-02-07T14:23:42Z

After we merge this we should probably update ps-spec-* libs

purescript-spec-discovery because now one can interleave some monadic effect in test generation the discover can have signature like this: discover :: MoandEffect m => String -> SpecT g i m Unit.
purescript-spec-reporter-xunit should be easy to update.
purescript-spec-quickcheck doesn't even depend on ps-spec and I guess needs no change.
purescript-spec-mocha updating this might not be trivial as we support parallel execution of tests but mocha doesn't. as I understand the main (if not only) benefit of using mocha for user is ability to run tests both in browser and node. it can be accomplished by striping suing console.log instead of process.stdout.write (this article can be useful for using colors etc in browser console output). so what i propose is to deprecate that and add make our built in reporters work in browser.

Let me know what others think about this.

safareli · 2019-02-07T14:28:42Z

Also do you think it's worth releasing this changes as v4.0.0-rc1 (maybe people can play with it and get some feedback) or we should go straight to v4?

safareli · 2019-02-28T11:20:01Z

🏓 ping

owickstrom · 2019-03-19T22:20:04Z

I'm good with these last changes! And yeah, I think releasing an RC would be good.

owickstrom · 2019-03-19T22:20:24Z

@felixmulder you good with merging this?

owickstrom · 2019-03-20T05:33:07Z

👏

safareli · 2019-03-22T15:17:01Z

Made a release v4.0.0-rc1

safareli added 7 commits January 10, 2019 17:53

add seq/par

4e06727

add hoist and example/tests

daf6b33

revert whitespace

47605c5

add docs for parallel

37e278b

src compiles

0681a83

add hoist and some cleanup

114f8bb

safareli force-pushed the hook branch from 876fae7 to 114f8bb Compare January 10, 2019 14:01

safareli mentioned this pull request Jan 10, 2019

WIP: Implement hooks #52

Closed

6 tasks

safareli added 2 commits January 11, 2019 11:21

moved Aff tests into Predicates

2ad5c0c

as they are not Aff specific any more

add Path to Runner.Events

b356736

now consoleReporter and specReporter need to coordinate tests which are running in parallel

safareli added 8 commits January 14, 2019 15:34

adopt ConsoleReporter to parallel execution

12168c8

some cleanup

ec4bda6

update reporter with copy&paste

1232a9a

put Speed and Duration in Success

1459670

merge Fail and Pass into TestEnd

e1df5dd

optimize redraw

f433b4d

use Ansi codes for screan clearing

8ae1a00

optimize redraw

58d4f9e

safareli added 6 commits January 15, 2019 17:02

no redraw

d22706f

only log things to console when execution of test is finished so we don't need to do redraw

refactor styling

6baa794

restore log in hoistSpec test

d6bcba2

use List instead of Array

51d385a

move indent to Style

66b67e6

remove defaultUpdate

94a08ec

safareli added 9 commits January 16, 2019 11:23

adopt Using hooks section from hspec

30d68f6

use purescripts-now and Milliseconds instead of Int

00f8589

update Reporter exports

239491b

use absurd for handling void case

4bf23d7

remove reduce run,run',runSpec,runSpec' to just runSpec and runSpecM

2884c0c

with `exit: false` and you can get test results with `[]` as reporters you can get no reporting so removed runSpec,runSpec' and renamed run,run' to runSpec and runSpecM

remove extra ()

21f2a98

fix warning

4cbccc2

some documentation updates

708bc0e

owickstrom requested changes Feb 1, 2019

View reviewed changes

owickstrom mentioned this pull request Feb 2, 2019

Custom compile warning for itOnly and describeOnly #63

Closed

safareli added 3 commits February 7, 2019 17:17

restore run with warning

375b9d7

add collect

c3780f0

allow chaning of m in hoistSpec and mapSpecTree

0d50c9c

fix warning

85c503f

owickstrom approved these changes Mar 19, 2019

View reviewed changes

felixmulder merged commit 003ef97 into purescript-spec:master Mar 20, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add seq/par and hook #83

add seq/par and hook #83

safareli commented Jan 10, 2019 •

edited

safareli commented Jan 10, 2019

felixmulder commented Jan 10, 2019

safareli commented Jan 11, 2019

safareli commented Jan 11, 2019

safareli commented Jan 15, 2019 •

edited

felixmulder commented Jan 15, 2019

felixmulder commented Jan 15, 2019

safareli commented Jan 16, 2019

safareli commented Jan 16, 2019

owickstrom commented Jan 26, 2019

owickstrom left a comment

owickstrom Feb 1, 2019

owickstrom Feb 1, 2019

safareli Feb 7, 2019

safareli commented Feb 7, 2019

safareli commented Feb 7, 2019

safareli commented Feb 7, 2019

safareli commented Feb 28, 2019

owickstrom commented Mar 19, 2019

owickstrom commented Mar 19, 2019

owickstrom commented Mar 20, 2019

safareli commented Mar 22, 2019

add seq/par and hook #83

add seq/par and hook #83

Conversation

safareli commented Jan 10, 2019 • edited

safareli commented Jan 10, 2019

felixmulder commented Jan 10, 2019

safareli commented Jan 11, 2019

safareli commented Jan 11, 2019

safareli commented Jan 15, 2019 • edited

But, :(

felixmulder commented Jan 15, 2019

felixmulder commented Jan 15, 2019

safareli commented Jan 16, 2019

safareli commented Jan 16, 2019

owickstrom commented Jan 26, 2019

owickstrom left a comment

Choose a reason for hiding this comment

owickstrom Feb 1, 2019

Choose a reason for hiding this comment

owickstrom Feb 1, 2019

Choose a reason for hiding this comment

safareli Feb 7, 2019

Choose a reason for hiding this comment

safareli commented Feb 7, 2019

safareli commented Feb 7, 2019

safareli commented Feb 7, 2019

safareli commented Feb 28, 2019

owickstrom commented Mar 19, 2019

owickstrom commented Mar 19, 2019

owickstrom commented Mar 20, 2019

safareli commented Mar 22, 2019

safareli commented Jan 10, 2019 •

edited

safareli commented Jan 15, 2019 •

edited