Skip to content

Commit

Permalink
Fix time duration formatting as per SI (#9765)
Browse files Browse the repository at this point in the history
  • Loading branch information
getsnoopy committed May 2, 2020
1 parent ba962e7 commit 71a048e
Show file tree
Hide file tree
Showing 18 changed files with 109 additions and 27 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -89,6 +89,7 @@

### 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))
- `[expect]` Restore support for passing functions to `toHaveLength` matcher ([#9796](https://github.com/facebook/jest/pull/9796))
- `[jest-changed-files]` `--only-changed` should include staged files ([#9799](https://github.com/facebook/jest/pull/9799))
- `[jest-circus]` Throw on nested test definitions ([#9828](https://github.com/facebook/jest/pull/9828))
Expand Down
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
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
Expand Up @@ -142,7 +142,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 @@ -194,7 +194,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
2 changes: 1 addition & 1 deletion e2e/__tests__/overrideGlobals.test.ts
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
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 dedent = require('dedent');
Expand Down Expand Up @@ -139,7 +139,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
4 changes: 2 additions & 2 deletions packages/jest-console/src/BufferedConsole.ts
Expand Up @@ -9,7 +9,7 @@ import assert = require('assert');
import {Console} from 'console';
import {format} from 'util';
import chalk = require('chalk');
import {ErrorWithStack} from 'jest-util';
import {ErrorWithStack, formatTime} from 'jest-util';
import type {
ConsoleBuffer,
LogCounters,
Expand Down Expand Up @@ -154,7 +154,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
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 @@ -131,7 +131,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
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
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
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
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
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
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
@@ -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
@@ -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
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
Expand Up @@ -22,6 +22,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
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 71a048e

Please sign in to comment.