Skip to content

Commit

Permalink
Fix time duration formatting as per SI
Browse files Browse the repository at this point in the history
This commit fixes the formatting of time durations to have a spcae
between the quantity and the unit symbol which is compliant with
the SI. It also consolidates all of the time duration formatting
code into formatTime() in jest-util to keep the code DRY and so
that future updates are easy.
  • Loading branch information
getsnoopy committed Apr 4, 2020
1 parent 7e5e422 commit a6e5573
Show file tree
Hide file tree
Showing 19 changed files with 111 additions and 27 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

### Fixes

- `[jest-circus, jest-console, jest-jasmine2, jest-reporters, jest-util, pretty-format]` Fix time durating formatting and consolidate time formatting code ([#9765](https://github.com/facebook/jest/pull/9765))

### Chore & Maintenance

- `[@jest/transform]` Expose type `CacheKeyOptions` for `getCacheKey` ([#9762](https://github.com/facebook/jest/pull/9762))
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ PASS __tests__/clear_cache.test.js
Test Suites: 1 passed, 1 total
Tests: 1 passed, 1 total
Snapshots: 0 total
Time: 0.232s, estimated 1s
Time: 0.232 s, estimated 1 s
Ran all test suites.
```

Expand Down
4 changes: 2 additions & 2 deletions e2e/Utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ export const copyDir = (src: string, dest: string) => {

export const replaceTime = (str: string) =>
str
.replace(/\d*\.?\d+m?s/g, '<<REPLACED>>')
.replace(/\d*\.?\d+ m?s\b/g, '<<REPLACED>>')
.replace(/, estimated <<REPLACED>>/g, '');

// Since Jest does not guarantee the order of tests we'll sort the output.
Expand Down Expand Up @@ -192,7 +192,7 @@ export const extractSummary = (stdout: string) => {
const rest = stdout
.replace(match[0], '')
// remove all timestamps
.replace(/\s*\(\d*\.?\d+m?s\)$/gm, '');
.replace(/\s*\(\d*\.?\d+ m?s\b\)$/gm, '');

return {
rest: rest.trim(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ PASS __tests__/console.test.js
14 | });
15 |
at BufferedConsole.log (../../packages/jest-console/build/BufferedConsole.js:199:10)
at BufferedConsole.log (../../packages/jest-console/build/BufferedConsole.js:209:10)
at log (__tests__/console.test.js:12:13)
`;
2 changes: 1 addition & 1 deletion e2e/__tests__/overrideGlobals.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ test('overriding native promise does not freeze Jest', () => {
});

test('has a duration even if time is faked', () => {
const regex = /works well \((\d+)ms\)/;
const regex = /works well \((\d+) ms\)/;
const {stderr} = runJest('override-globals', ['--verbose']);

expect(stderr).toMatch(regex);
Expand Down
4 changes: 2 additions & 2 deletions packages/jest-circus/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
*/

import type {Circus} from '@jest/types';
import {convertDescriptorToString} from 'jest-util';
import {convertDescriptorToString, formatTime} from 'jest-util';
import isGeneratorFn from 'is-generator-fn';
import co from 'co';
import StackUtils = require('stack-utils');
Expand Down Expand Up @@ -138,7 +138,7 @@ export const describeBlockHasTests = (
describe.tests.length > 0 || describe.children.some(describeBlockHasTests);

const _makeTimeoutMessage = (timeout: number, isHook: boolean) =>
`Exceeded timeout of ${timeout}ms for a ${
`Exceeded timeout of ${formatTime(timeout)} for a ${
isHook ? 'hook' : 'test'
}.\nUse jest.setTimeout(newTimeout) to increase the timeout value, if this is a long-running test.`;

Expand Down
3 changes: 2 additions & 1 deletion packages/jest-console/src/BufferedConsole.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import assert = require('assert');
import {Console} from 'console';
import {format} from 'util';
import {formatTime} from 'jest-util';
import chalk = require('chalk');
import {SourceMapRegistry, getCallsite} from '@jest/source-map';
import type {
Expand Down Expand Up @@ -150,7 +151,7 @@ export default class BufferedConsole extends Console {
if (startTime) {
const endTime = new Date();
const time = endTime.getTime() - startTime.getTime();
this._log('time', format(`${label}: ${time}ms`));
this._log('time', format(`${label}: ${formatTime(time)}`));
delete this._timers[label];
}
}
Expand Down
4 changes: 2 additions & 2 deletions packages/jest-console/src/CustomConsole.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import assert = require('assert');
import {format} from 'util';
import {Console} from 'console';
import chalk = require('chalk');
import {clearLine} from 'jest-util';
import {clearLine, formatTime} from 'jest-util';
import type {LogCounters, LogMessage, LogTimers, LogType} from './types';

type Formatter = (type: LogType, message: LogMessage) => string;
Expand Down Expand Up @@ -132,7 +132,7 @@ export default class CustomConsole extends Console {
if (startTime) {
const endTime = new Date().getTime();
const time = endTime - startTime.getTime();
this._log('time', format(`${label}: ${time}ms`));
this._log('time', format(`${label}: ${formatTime(time)}`));
delete this._timers[label];
}
}
Expand Down
2 changes: 1 addition & 1 deletion packages/jest-jasmine2/src/__tests__/queueRunner.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ describe('queueRunner', () => {
expect(onException).toHaveBeenCalled();
// i.e. the `message` of the error passed to `onException`.
expect(onException.mock.calls[0][0].message).toEqual(
'Timeout - Async callback was not invoked within the 0ms timeout ' +
'Timeout - Async callback was not invoked within the 0 ms timeout ' +
'specified by jest.setTimeout.',
);
expect(fnTwo).toHaveBeenCalled();
Expand Down
5 changes: 3 additions & 2 deletions packages/jest-jasmine2/src/queueRunner.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* LICENSE file in the root directory of this source tree.
*/

import {formatTime} from 'jest-util';
// @ts-ignore ignore vendor file
import PCancelable from './PCancelable';
import pTimeout from './pTimeout';
Expand Down Expand Up @@ -71,8 +72,8 @@ export default function queueRunner(options: Options) {
() => {
initError.message =
'Timeout - Async callback was not invoked within the ' +
timeoutMs +
'ms timeout specified by jest.setTimeout.';
formatTime(timeoutMs) +
' timeout specified by jest.setTimeout.';
initError.stack = initError.message + initError.stack;
options.onException(initError);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ exports[`snapshots all have results (after update) 1`] = `
<bold>Test Suites: </><bold><red>1 failed</></>, 1 total
<bold>Tests: </><bold><red>1 failed</></>, 1 total
<bold>Snapshots: </><bold><red>1 failed</></>, <bold><green>1 removed</></>, <bold><green>1 file removed</></>, <bold><green>1 updated</></>, <bold><green>1 written</></>, <bold><green>2 passed</></>, 2 total
<bold>Time:</> 0.01s
<bold>Time:</> 0.01 s
<dim>Ran all test suites</><dim>.</>
"
`;
Expand All @@ -31,7 +31,7 @@ exports[`snapshots all have results (no update) 1`] = `
<bold>Test Suites: </><bold><red>1 failed</></>, 1 total
<bold>Tests: </><bold><red>1 failed</></>, 1 total
<bold>Snapshots: </><bold><red>1 failed</></>, <bold><yellow>1 obsolete</></>, <bold><yellow>1 file obsolete</></>, <bold><green>1 updated</></>, <bold><green>1 written</></>, <bold><green>2 passed</></>, 2 total
<bold>Time:</> 0.01s
<bold>Time:</> 0.01 s
<dim>Ran all test suites</><dim>.</>
"
`;
Expand All @@ -43,7 +43,7 @@ exports[`snapshots needs update with npm test 1`] = `
<bold>Test Suites: </><bold><red>1 failed</></>, 1 total
<bold>Tests: </><bold><red>1 failed</></>, 1 total
<bold>Snapshots: </><bold><red>2 failed</></>, 2 total
<bold>Time:</> 0.01s
<bold>Time:</> 0.01 s
<dim>Ran all test suites</><dim>.</>
"
`;
Expand All @@ -55,7 +55,7 @@ exports[`snapshots needs update with yarn test 1`] = `
<bold>Test Suites: </><bold><red>1 failed</></>, 1 total
<bold>Tests: </><bold><red>1 failed</></>, 1 total
<bold>Snapshots: </><bold><red>2 failed</></>, 2 total
<bold>Time:</> 0.01s
<bold>Time:</> 0.01 s
<dim>Ran all test suites</><dim>.</>
"
`;
3 changes: 2 additions & 1 deletion packages/jest-reporters/src/get_result_header.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import type {Config} from '@jest/types';
import type {TestResult} from '@jest/test-result';
import chalk = require('chalk');
import {formatTime} from 'jest-util';
import {formatTestPath, printDisplayName} from './utils';
import terminalLink = require('terminal-link');

Expand Down Expand Up @@ -47,7 +48,7 @@ export default (

const testDetail = [];
if (runTime !== null && runTime > 5) {
testDetail.push(LONG_TEST_COLOR(runTime + 's'));
testDetail.push(LONG_TEST_COLOR(formatTime(runTime, 0)));
}

if (result.memoryUsage) {
Expand Down
8 changes: 4 additions & 4 deletions packages/jest-reporters/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import type {Config} from '@jest/types';
import type {AggregatedResult} from '@jest/test-result';
import chalk = require('chalk');
import slash = require('slash');
import {pluralize} from 'jest-util';
import {formatTime, pluralize} from 'jest-util';
import type {SummaryOptions} from './types';

const PROGRESS_BAR_WIDTH = 40;
Expand Down Expand Up @@ -185,11 +185,11 @@ const renderTime = (runTime: number, estimatedTime: number, width: number) => {
// If we are more than one second over the estimated time, highlight it.
const renderedTime =
estimatedTime && runTime >= estimatedTime + 1
? chalk.bold.yellow(runTime + 's')
: runTime + 's';
? chalk.bold.yellow(formatTime(runTime, 0))
: formatTime(runTime, 0);
let time = chalk.bold(`Time:`) + ` ${renderedTime}`;
if (runTime < estimatedTime) {
time += `, estimated ${estimatedTime}s`;
time += `, estimated ${formatTime(estimatedTime, 0)}`;
}

// Only show a progress bar if the test run is actually going to take
Expand Down
6 changes: 4 additions & 2 deletions packages/jest-reporters/src/verbose_reporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import type {
TestResult,
} from '@jest/test-result';
import chalk = require('chalk');
import {specialChars} from 'jest-util';
import {formatTime, specialChars} from 'jest-util';
import type {Test} from './types';
import DefaultReporter from './default_reporter';

Expand Down Expand Up @@ -107,7 +107,9 @@ export default class VerboseReporter extends DefaultReporter {

private _logTest(test: AssertionResult, indentLevel: number) {
const status = this._getIcon(test.status);
const time = test.duration ? ` (${test.duration.toFixed(0)}ms)` : '';
const time = test.duration
? ` (${formatTime(Math.round(test.duration))})`
: '';
this._logLine(status + ' ' + chalk.dim(test.title + time), indentLevel);
}

Expand Down
52 changes: 52 additions & 0 deletions packages/jest-util/src/__tests__/formatTime.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

import formatTime from '../formatTime';

it('defaults to milliseconds', () => {
expect(formatTime(42)).toBe('42 ms');
});

it('formats seconds properly', () => {
expect(formatTime(42, 0)).toBe('42 s');
});

it('formats milliseconds properly', () => {
expect(formatTime(42, -3)).toBe('42 ms');
});

it('formats microseconds properly', () => {
expect(formatTime(42, -6)).toBe('42 μs');
});

it('formats nanoseconds properly', () => {
expect(formatTime(42, -9)).toBe('42 ns');
});

it('interprets lower than lowest powers as nanoseconds', () => {
expect(formatTime(42, -12)).toBe('42 ns');
});

it('interprets higher than highest powers as seconds', () => {
expect(formatTime(42, 3)).toBe('42 s');
});

it('interprets non-multiple-of-3 powers as next higher prefix', () => {
expect(formatTime(42, -4)).toBe('42 ms');
});

it('formats the quantity properly when pad length is lower', () => {
expect(formatTime(42, -3, 1)).toBe('42 ms');
});

it('formats the quantity properly when pad length is equal', () => {
expect(formatTime(42, -3, 2)).toBe('42 ms');
});

it('left pads the quantity properly when pad length is higher', () => {
expect(formatTime(42, -3, 5)).toBe(' 42 ms');
});
22 changes: 22 additions & 0 deletions packages/jest-util/src/formatTime.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
/**
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

export default function formatTime(
time: number,
prefixPower: number = -3,
padLeftLength: number = 0,
): string {
const prefixes = ['n', 'μ', 'm', ''];
const prefixIndex = Math.max(
0,
Math.min(
Math.trunc(prefixPower / 3) + prefixes.length - 1,
prefixes.length - 1,
),
);
return `${String(time).padStart(padLeftLength)} ${prefixes[prefixIndex]}s`;
}
1 change: 1 addition & 0 deletions packages/jest-util/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@ export {default as replacePathSepForGlob} from './replacePathSepForGlob';
export {default as testPathPatternToRegExp} from './testPathPatternToRegExp';
import * as preRunMessage from './preRunMessage';
export {default as pluralize} from './pluralize';
export {default as formatTime} from './formatTime';

export {preRunMessage, specialChars};
1 change: 1 addition & 0 deletions packages/pretty-format/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
"@types/react-is": "^16.7.1",
"@types/react-test-renderer": "*",
"immutable": "4.0.0-rc.9",
"jest-util": "^25.2.6",
"react": "*",
"react-dom": "*",
"react-test-renderer": "*"
Expand Down
7 changes: 4 additions & 3 deletions packages/pretty-format/perf/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
const util = require('util');
const chalk = require('chalk');
const React = require('react');
const {formatTime} = require('jest-util');
const ReactTestRenderer = require('react-test-renderer');
const prettyFormat = require('../build');
const ReactTestComponent = require('../build/plugins/ReactTestComponent');
Expand Down Expand Up @@ -78,13 +79,13 @@ function test(name, value, ignoreResult, prettyFormatOpts) {
let message = current.name;

if (current.time) {
message += ' - ' + String(current.time).padStart(6) + 'ns';
message += ' - ' + formatTime(current.time, -9, 6);
}
if (current.total) {
message +=
' - ' +
current.total / NANOSECONDS +
's total (' +
formatTime(current.total / NANOSECONDS, 0) +
' total (' +
TIMES_TO_RUN +
' runs)';
}
Expand Down

0 comments on commit a6e5573

Please sign in to comment.