"# time=..." directive

[isaacs]

It'd be nice to have a standard way to pass test duration (ie, the amount of time it took for a test to complete), as well as the total time for the set.

I've been experimenting with this approach:

1..3
ok 1 flurb is fast # time=1.234ms
ok 2 blub is slow # time=432.543ms
not ok 3 slow failure # time=501.3ms
# failed 1 of 3 tests
# time=942.5ms

Of course, it could be put in a yaml diag as well, but since diags are typically not printed for passing tests, that doesn't help in the typical case where you have lots of passing tests, but want to optimize just the slow ones.

The directive would look like /time=((?:[1-9][0-9]*|0)(?:\.[0-9]+)?)(ms|s)/i where $1 is the number and $2 is the label (either seconds or ms).

I don't expect that this would conflict with todo or skip directives, since todo/skip test points are usually near-immediate anyway.

[jonathanKingston]

I would be +1 for this if YAML was being replaced.

I think the issues mostly are:

• YAML for passing tests is mostly ignored
• There is no standardised YAML format for this data

By adding a new directive both consumer and producer would have to be updated anyway, where by the previous two points could be addressed.

As much as I really don't like the YAML usage, I don't see the advantage to further complexity.

[isaacs]

Yaml for passing tests is excessively noisy; many test runners don't even bother printing it. While of course you can pipe tap to a consumer that does something clever with it, if we are going to say that tap is a machine-readable format not designed for humans, then streaming json or csv would be much better.

If we say that one of the fields in yamlish is going to have some meaning in the specification, then that's a pretty big rubicon to cross. I'm not saying I'm against that, but it is an addition that's much broader in scope. A test runner today could well be using time: ... to mean a specific thing today, other than the test duration. (For example, a timestamp of an object, when the test was run, etc.) On the other hand, it's pretty well guaranteed that no tap producer is using ok <number> <name> # time=...ms in their output today, since it would serve no purpose.

That's why I proposes that we leave yaml as being application specific generic test information, and add time as a directive. It's a much cheaper addition.

[beatgammit]

I'm a fan of that syntax, and I think it should be allowed anywhere in the stream so a long-running test can denote progress. For example:

ok 1 a short test # time=10ms
# time=10s
# time=20s
# time=30s
# time=40s
# time=50s
# time=60s
ok 2 Long running test # time=59.09ms

This way the consumer can differentiate between a hung test and a long-running test. This would be completely optional and meaning should be left to the producer.

Also related, it might be nice to have an expected time (e.g. # duration=60). at the beginning of the test (or in the middle as well) so something like a progress-bar could be implemented for a long-running test-suite (we have one that takes a couple hours). The consumer could estimate this after enough data has been gathered, but I think it's useful metadata for the producer to provide since it knows which (if any) tests have been skipped beforehand.

If this second part belongs in a separate proposal, let me know. I don't want to derail this conversation.

[jonathanKingston]

@isaacs @beatgammit has highlighted my distaste for this completely. I was about to say that quickly more parameters will be requested and each line will quickly become to look like a URL string with 10 get parameters.

This way the consumer can differentiate between a hung test and a long-running test. This would be completely optional and meaning should be left to the producer.

Already there is a section for random garbage the producer has, it's the YAML section.

[isaacs]

@jonathanKingston I think that the disconnect is happening here:

Already there is a section for random garbage the producer has, it's the YAML section.

Yes, that is a place for "random garbage". That's precisely why this non-random non-garbage does not belong there.

1. Yaml is almost never shown for passing tests, for good reason. It's visually noisy, and can be expensive to generate. However, timing data should be shown for passing tests.
2. Yaml key/value pairs are currently assigned no particular meaning. Saying that the "time" field has some special proscribed meaning is a significant step towards specifying yaml key semantics. I'm not saying that's a bad thing, but it certainly is another thing.
3. @beatgammit's suggestion is not adequately served by yaml, since yaml comes after the test point to which it refers, and any indication of a long-running test should come before the test point line. It would be problematic because it'd be confused with the # time=... following a test set.
4. @beatgammit I think that adding # duration=... for the intended time is not a great idea. It's better for the test framework itself to set a timeout, and then fail the test after some time. (node-tap, tape, and jtap all have support for this, I'm not sure of TAP::Test, Test::More et al.)

Regarding the concern that directives could end up a grab-bag of query params, I think that's largely mitigated by the fact that every directive has specific semantic meaning, and they cannot be combined. For example, ok 1 some name # SKIP TODO would be interpreted as a "skipped" test with the reason for skipping printed as "TODO". That is, it would be { skip: "TODO" }, not { skip: true, todo: true }.

Since this is already perfectly valid TAP 13, I'm going to use it in node-tap 1.x, since we desperately need to start reporting timing values.

[beatgammit]

@beatgammit I think that adding # duration=... for the intended time is not a great idea. It's better for the test framework itself to set a timeout, and then fail the test after some time. (node-tap, tape, and jtap all have support for this, I'm not sure of TAP::Test, Test::More et al.)

I didn't intend for this to have anything to do with failing a test that takes too long, I just want a way for the producer to let the consumer know how long it expects the test to run. This seems like it's derailing the current conversation, so I propose we drop the whole issue of expected run-time.

@beatgammit's suggestion is not adequately served by yaml, since yaml comes after the test point to which it refers

That's exactly what I was trying to say. I think that the # time=10s stand-alone statements are a separate idea from the time a specific test took (ok 1 Test # time=2s), so they could be included while still having the time be reported in YAML.

I do like the consistency of this rather than having it in the YAML, but I do understand @jonathanKingston's fear of tons of meta information on the test line. I think that anything more than the time for the test would make it too difficult to read.

I honestly don't care which approach (YAML or # time=1s) is taken, just that it gets standardized.

[jonathanKingston]

Yes, that is a place for "random garbage". That's precisely why this non-random non-garbage does not belong there

@isaacs I was addressing the producer owns the meaning of the properties as raised by @beatgammit - this is an issue and yes YAML suffers from this. I'm not convinced by anything dictated by a producer rather than the standard.

Since this is already perfectly valid TAP 13, I'm going to use it in node-tap 1.x, since we desperately need to start reporting timing values.

There in lies the issue with the whole standard. It's lead by usage which isn't a standard at all. Comments were never designed to have directives within them, much like the craziness of using 'use strict' in JavaScript it is just about ok, if that became 'use x,y,z=12' we are starting to add a language feature into a comment.

Namespaced YAML has been suggested many times before for the exact usecase here which is a far better idea than adding yet another place to metadata that won't scale very well.

There is also another issue open to add meaning to the YAML document #8

If a separate conversation needs to be opened about a better place for meta date I would be much happier with that. One of the issues I have with this is that it doesn't scale for other meta data tests might need.

ok 1 Test happened for a certain item # time=2s duration=12 language=php server=10.1.1.102 lang=en

I would urge you not just to implement yet further changes to the format until we can hammer out some consensus. #17

[cjihrig]

Sorry for bumping such an old issue. Having the test duration as part of the spec would be helpful because TAP consumers often want to display test durations, but different TAP producers do generate this in a consistent format - directive vs. YAML, differing names, different units of time.