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

Fix time duration formatting as per SI #9765

Merged
merged 1 commit into from May 2, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -9,6 +9,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-each]` `each` will throw an error when called with too many arguments ([#9818](https://github.com/facebook/jest/pull/9818))
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 @@ -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
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 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
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 @@ -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
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 @@ -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
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