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

Could use a diag() Method #91

Open
theory opened this issue Jun 20, 2014 · 10 comments · May be fixed by #99
Open

Could use a diag() Method #91

theory opened this issue Jun 20, 2014 · 10 comments · May be fixed by #99

Comments

@theory
Copy link

theory commented Jun 20, 2014

It's pretty typical for TAP implementations to offer an interface for emitting diagnostic messages as TAP comments. Examples:

I tried to figure out where to plug one in to submit a patch, but the path to the output handle was over my head, so I used console.log() for my immediate use. Would be nice to have a canonical place for it.

@Raynos
Copy link
Collaborator

Raynos commented Jun 20, 2014

👍

We should also look at Test::More or Test.More and take more inspiration from PERL.

@theory
Copy link
Author

theory commented Jun 20, 2014

Test.More is a pretty direct port of the Perl module, FWIW. I wrote it a long time ago…

@Raynos
Copy link
Collaborator

Raynos commented Jun 20, 2014

i think the way to add this is to add it to lib/test.js as a .diag() method and have it emit a diag event

Then in lib/results.js we just need to listen to diag events stream.queue() the correct TAP string format.

@andrewdeandrade
Copy link
Contributor

FWIW, instead of copying the behavior of other implementations verbatim, there is renewed discussion around finalizing the TAP IETF RFC and adding some proposed features to version 14. @Ovid has been doing new work in this space in the perl community.

Check out the new discussions from the last few days here:
http://www.ietf.org/mail-archive/web/tap/current/maillist.html

that being said, I totally agree that we could use a diag() method (since it is in version 13 of the spec)

@andrewdeandrade
Copy link
Contributor

Also, related to this is the TAP Y/J specification that is worth looking at. Right now, version 13 only specifies YAML matter between --- and ..., but getting JSON added for version 14 of the spec would be valuable for the JavaScript community. If you haven't yet seen it, take a look at the following two links:

TestAnything/testanything.github.io#6
https://github.com/rubyworks/tapout/wiki/TAP-Y-J-Specification

@Raynos
Copy link
Collaborator

Raynos commented Aug 14, 2014

@malandrew @substack

We should do a tape hack day some time to fix up various little things (with lots of those tests)

@andrewdeandrade andrewdeandrade linked a pull request Aug 15, 2014 that will close this issue
@andrewdeandrade
Copy link
Contributor

@Raynos I would actually love to actually spend time with both node-tap and tape to remove duplicated efforts between the two and still keep the browser compatibility. For example, my most recent pull request adds diag() to tape, but I would need to author another pull request to add that feature to node-tap. AFAICT, tape can almost entirely replace tap-assert.js and tap-test.js in isaacs/node-tap.

Consolidating the efforts, cleaning up these outstanding issues and adding better documentation to node-tap would be really valuable. I'd really like to see the TAP protocol libraries in the npm ecosystem be the best testing choice out there. There is so much to gain community wise by getting libraries (current and future) standardized around TAP. It's possible to go even beyond what CPAN Testers does.

One thing I'd really like to explore is using TAP for local "CI assistance". i.e. If I made library A and libraries B and C depend on A, it would be interesting to not only run my tests, but also the tests of my dependents, diffing the test results from those dependents using the previous version of A and the current version of A. This is super valuable information for helping the maintainers of B and C upgrade their dependencies and helping library builders know decide whether to (a) do a minor or possibly even a major semver bump because their changes breaks their dependents, or (b) inform the maintainers of their dependents that their semver ranges are too loose.

@theory
Copy link
Author

theory commented Aug 15, 2014

@malandrew +1!

@Ovid
Copy link

Ovid commented Aug 15, 2014

While a diag() method is useful, you might also want to consider the extremely useful explain() function. explain() is similar to diag(), but it only runs in when the test harness is running in verbose mode. This can be invaluable when you don't want your test output cluttered with a bunch of diagnostics that don't really apply to the pass/fail nature of the entire test suite. explain() is used when you have things like this (pseudo-code):

client.get_ok("/path/to/a/web/page")

  || explain( { status: client.status, page: client.html.visible } );

In the above example, if the get_ok() call fails, explain() dumps a data structure showing the client status and the visible html (e.g., what a human would see). That could be a huge amount of output, but for a full test suite run, all that extra output is noisy and can make it hard to see what you're looking for. If tests are run in non-verbose mode, calling explain() is a no-op.

For diag(), there are times that you want diagnostic information visible even when the full test suite is run. For example:

diag("The following tests are very fragile. Fix them!");

... fragile tests follow

// or

var geo_server = server.connect.authenticate(credentials);
if (!geo_server) {
  diag("Something's wrong with the geo_server: " + geo_server.errstr);
  return;
}
In both of those examples, diag() is being used to communicate information that should not be overlooked.

Best,

Ovid

IT consulting, training, international recruiting
       http://www.allaroundtheworld.fr/.
Buy my book! - http://bit.ly/beginning_perl
Live and work overseas - http://www.overseas-exile.com/

On Tuesday, 12 August 2014, 23:07, Andrew de Andrade [email protected] wrote:

FWIW, instead of copying other implementations blindly, there is renewed discussion around finalizing the TAP IETF RFC and adding some proposed features to version 14. @Ovid has been doing new work in this space in the perl community.
Check out the new discussions from the last few days here:
http://www.ietf.org/mail-archive/web/tap/current/maillist.html

Reply to this email directly or view it on GitHub.

@andrewdeandrade
Copy link
Contributor

@Ovid I really like the idea of an explain() method. Will investigate.

I also agree with your description of when diag() should be used, since it most closely matches one of my mid to long term use cases: reporting benchmark details.

i.e. I want an assertion to pass if, over N runs, it completes faster than than X milliseconds with a standard deviation not greater than Y% of X. In some cases I may want to only show performance metrics when the test fails. In other cases, where I am trying to determine performance gains between versions, I might want to always display perf yaml data so that the figures can be compared between runs to make sure your library is alway getting faster.

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 a pull request may close this issue.

4 participants