search

jberger / Mojo-Phantom

PhantomJS client-side testing for Mojolicious apps
Other
13 stars 6 forks source link

Add no_subtest option to Test::Mojo::Role::Phantom #1

Closed ksmadsen closed 8 years ago

ksmadsen commented 8 years ago

Calling phantom_ok with the no_subtest option set will cause phantom_ok to not produce a subtest, but instead perform all the tests within the current test-context.

This is useful if e.g. the TAP::Formatter::JUnit formatter is in use, as it doesn't support subtests.

jberger commented 8 years ago

I feel that the subtest is an important part of the implementation. When someone runs one _ok method I'd expect that one test is generated. Since there may indeed be many tests run from the javascript side (which the phantom_ok method can't know beforehand) then this is essentially the definition of a subtest: one test method that internally runs more tests and whose pass/fail status is the result of the aggregation of internal tests.

Were that the only consideration I might still give the option, but actually as soon as Test2 finally goes stable I was hoping to use it even more deeply to fix the location of the test (which currently emits from inside the IOLoop). I don't know how the additional option would interact with that goal.

Finally, if I were to accept this patch, it would have to be modified to set $t->success in any case.

ksmadsen commented 8 years ago

I understand your concerns, and agree with them.

It was just easier to remove the subtest from Test::Mojo::Role::Phantom, than to fix TAP::Formatter::JUnit to give us the output we need ;-)

I'll withdraw my pull request.

jberger commented 8 years ago

And yes, if TAP::Formatter::JUnit could be fixed to handle subtests that a much better solution. I use subtests in many of my modules, it would be nice if it could handle them.