test: change duration_ms to duration #7133
Conversation
mscdex
added
test
|
LGTM |
1 similar comment
|
LGTM |
|
@jasnell I can't imagine how this could cause tests to fail, but is it worth running a test-commit on it anyway? |
|
Of course :-): https://ci.nodejs.org/job/node-test-pull-request/2914/ |
| @@ -310,7 +310,7 @@ def HasRun(self, output): | |||
| (duration.seconds + duration.days * 24 * 3600) * 10**6) / 10**6 | |||
|
|
|||
| logger.info(' ---') | |||
| logger.info(' duration_ms: %d.%d' % (total_seconds, duration.microseconds / 1000)) | |||
| logger.info(' duration: %d.%d' % (total_seconds, duration.microseconds / 1000)) | |||
There was a problem hiding this comment.
should this be duration: %d.%ds? (where s denotes seconds)?
There was a problem hiding this comment.
That would definitely make sense.
|
Updated as per @Fishrock123's suggestion, PTAL. |
The test script (tools/test.py) logs duration as "duration_ms: x.y". This is confusing (as the duration is measured in seconds). New example output: duration: 0.212s
|
LGTM |
1 similar comment
|
LGTM |
The test script (tools/test.py) logs duration as "duration_ms: x.y". This is confusing (as the duration is measured in seconds). New example output: duration: 0.212s PR-URL: #7133 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
|
Landed in d413378 |
|
Reversion filed at #7214, sorry to be a party-pooper and for catching this late, but at least now it'll stick in more than just my head when it inevitably comes up in the future:
|
The test script (tools/test.py) logs duration as "duration_ms: x.y". This is confusing (as the duration is measured in seconds). New example output: duration: 0.212s PR-URL: #7133 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Checklist
Affected core subsystem(s)
test
Description of change
The test script (tools/test.py) logs duration as "duration_ms: x.y" this is confusing (as the duration is measured in seconds). This PR changes
duration_mstoduration. It's a small change, but theduration_mscauses a lot of confusion to people who are new to the tests.The only other use of
duration_msin the codebase is in test/cctest/test-cpu-profiler.cc which I don't think should be affected by this change.For background see @rvagg's comment - #2393 (comment)