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

Include assertions in TAP output #324

Closed
vadimdemedes opened this issue Dec 11, 2015 · 14 comments
Closed

Include assertions in TAP output #324

vadimdemedes opened this issue Dec 11, 2015 · 14 comments

Comments

@vadimdemedes
Copy link
Contributor

While I was working on TAP support, I realized reporters want to know the assertions for each test. At the moment, test title just duplicated as an assertion.

screen shot 2015-12-11 at 10 10 43 pm

As you see on the screenshot, it results in a double test title.
It happens in tap-spec, tap-difflet & tap-min reporters at least. Other reporters are ok.

So what we need now, is to collect information about each test's assertions (in Test class), so that we output meaningful results for TAP reporters. Plus, it would probably benefit future integrations (e.g. with text editors).

@vadimdemedes vadimdemedes added the enhancement new functionality label Dec 11, 2015
@jamestalmage
Copy link
Contributor

I am 👍 for this, but it takes us towards verbose mode, which was controversial.

Personally, I think once we get TAP in place, it's output should be verbose (showing every assertion, passing or failing). It should be the responsibility of reporters to suppress output they feel is overly verbose.

@vadimdemedes
Copy link
Contributor Author

AVA's default output won't change, this is only needed for TAP output. But yes, it is a step forward towards verbose mode.

@jamestalmage
Copy link
Contributor

AVA's default output won't change, this is only needed for TAP output

I was under the assumption that our default output would simply be our own custom TAP reporter.

@vadimdemedes
Copy link
Contributor Author

Nope, this issue does not make such assumptions, it's only about tracking assertions. Perhaps we should discuss this after TAP support is landed in master.

@jamestalmage
Copy link
Contributor

Perhaps we should discuss this after TAP support is landed in master.

Yep, actual code would be easier to discuss. Are you able to land TAP support without this issue being fixed?

@vadimdemedes
Copy link
Contributor Author

I can only submit a PR, but I wouldn't want to merge it to master without solving this issue.

@jamestalmage
Copy link
Contributor

let's do that.

@vadimdemedes vadimdemedes mentioned this issue Dec 12, 2015
6 tasks
@ariporad
Copy link
Contributor

@vdemedes, @jamestalmage: This would be really easy to do once #360 lands. So once #360 lands, I can take this on.

@ariporad
Copy link
Contributor

@jamestalmage, @vdemedes, @sindresourhus: ok, now that #360 is merged, should I start working on this?

@jamestalmage
Copy link
Contributor

I'd ask you to hold off. I already started pursuing that in #340, but put it on hold until tap support landed (done), and until @twada and I can find consensus on which parts power-assert should handle (pending).

#360 will certainly make this easy, so thanks for that.

If you can find something else to work on that would be great.

@ariporad
Copy link
Contributor

Sure. :)

@novemberborn
Copy link
Member

I realized reporters want to know the assertions for each test.

Do you mean t.deepEquals etc?

@novemberborn novemberborn changed the title Track assertions Include assertions in TAP output Apr 5, 2016
@jamestalmage
Copy link
Contributor

Do you mean t.deepEquals etc?

Yes. #340 was an initial stab at it. If you run AVA's test suite with tap test/*.js --reporter=spec you will see a line for every assertion, but it's not super helpful:

  fail-fast mode
    ✓ expect truthy value
    ✓ should be equivalent
    ✓ should be equal
    ✓ should be equal
    ✓ should match pattern provided

With power-assert we could make that way more helpful:

    ✓ t.truthy(api.options.failFast)
    ✓ t.same(tests, [{ok: true,title: 'first pass'}, {ok: false,title: 'second fail'}]);
    ✓ t.is(result.passCount, 1);
    ✓ t.is(result.failCount, 1);
    ✓ t.match(result.errors[0].error.message, /Test failed via t.fail()/);

@novemberborn
Copy link
Member

Ohhhh that makes sense. And is awesome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants