Skip to content

--generateCoverageForFiles #4246

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

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`--generateCoverageForFiles 1`] = `
"Test Suites: 2 passed, 2 total
Tests: 2 passed, 2 total
Snapshots: 0 total
Time: <<REPLACED>>
Ran all test suites.
"
`;

exports[`--generateCoverageForFiles 2`] = `
"

PASS __tests__/a.test.js
PASS __tests__/b.test.js"
`;

exports[`--generateCoverageForFiles 3`] = `
"----------|----------|----------|----------|----------|----------------|
File | % Stmts | % Branch | % Funcs | % Lines |Uncovered Lines |
----------|----------|----------|----------|----------|----------------|
All files | 100 | 100 | 100 | 100 | |
a.js | 100 | 100 | 100 | 100 | |
b.js | 100 | 100 | 100 | 100 | |
----------|----------|----------|----------|----------|----------------|
"
`;

exports[`--generateCoverageForFiles 4`] = `
"Test Suites: 1 passed, 1 total
Tests: 1 passed, 1 total
Snapshots: 0 total
Time: <<REPLACED>>
Ran all test suites matching /a.js/i.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is incorrect (also broken in master for --findRelatedTests)
i filed the issue #4247

"
`;

exports[`--generateCoverageForFiles 5`] = `
" PASS __tests__/a.test.js
✓ a

"
`;

exports[`--generateCoverageForFiles 6`] = `
"----------|----------|----------|----------|----------|----------------|
File | % Stmts | % Branch | % Funcs | % Lines |Uncovered Lines |
----------|----------|----------|----------|----------|----------------|
All files | 100 | 100 | 100 | 100 | |
a.js | 100 | 100 | 100 | 100 | |
----------|----------|----------|----------|----------|----------------|
"
`;
66 changes: 66 additions & 0 deletions integration_tests/__tests__/generate_coverage_for_files.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/**
* Copyright (c) 2014-present, Facebook, Inc. All rights reserved.
*
* This source code is licensed under the BSD-style license found in the
* LICENSE file in the root directory of this source tree. An additional grant
* of patent rights can be found in the PATENTS file in the same directory.
*
* @flow
*/

'use strict';

const path = require('path');
const os = require('os');
const skipOnWindows = require('../../scripts/skip_on_windows');
const {cleanup, writeFiles, extractSummary} = require('../utils');
const runJest = require('../runJest');

const DIR = path.resolve(os.tmpdir(), 'generate_coverage_for_files_test');

skipOnWindows.suite();

beforeEach(() => cleanup(DIR));
afterAll(() => cleanup(DIR));

test('--generateCoverageForFiles', () => {
writeFiles(DIR, {
'.watchmanconfig': '',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this really needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think if you have watchman installed on your computer (which we have at fb) it'll complain in stdout if this file is not present, which will in turn make it harder to snapshot the output :(

Copy link
Contributor Author

@boujeepossum boujeepossum Aug 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe it's worth creating a wraper function createJestProject() that'll add it by default.
but at the same time i'd rather keep it explicit

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm for explicitness here, it's easier not to loose the context

'__tests__/a.test.js': `
require('../a');
require('../b');
test('a', () => expect(1).toBe(1));
`,
'__tests__/b.test.js': `
require('../b');
test('b', () => expect(1).toBe(1));
`,
'a.js': 'module.exports = {}',
'b.js': 'module.exports = {}',
'package.json': JSON.stringify({jest: {testEnvironment: 'node'}}),
});

let stdout;
let stderr;

({stdout, stderr} = runJest(DIR, ['--coverage']));
let summary;
let rest;
({summary, rest} = extractSummary(stderr));
expect(summary).toMatchSnapshot();
expect(
rest.split('\n').map(s => s.trim()).sort().join('\n'),
).toMatchSnapshot();
// both a.js and b.js should be in the coverage
expect(stdout).toMatchSnapshot();

({stdout, stderr} = runJest(DIR, ['--generateCoverageForFiles', 'a.js']));

({summary, rest} = extractSummary(stderr));

expect(summary).toMatchSnapshot();
// should only run a.js
expect(rest).toMatchSnapshot();
// coverage should be collected only for a.js
expect(stdout).toMatchSnapshot();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

btw, not really needed here, but recently I came up with more descriptive snapshot testing such scenarios:

describe(('--generateCoverageForFiles') => {
  test('summary', () => expect(summary).toMatchSnapshot())
  test('rest', () => expect(rest).toMatchSnapshot())
  test('stdout', () => expect(stdout).toMatchSnapshot())
})

This gives more descriptive snapshot names, together with error messages when diffing later (it's easy to loose context in large snapshots, and names are easier to understand than checking which number is for which snap)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait.. are you running the subprocess outsude of test?
i agree that snapshots are hard to track!

});
1 change: 1 addition & 0 deletions integration_tests/__tests__/list_tests.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const runJest = require('../runJest');
const path = require('path');

const testRootDir = path.resolve(__dirname, '..', '..');
// $FlowFixMe
expect.addSnapshotSerializer({
print: val => val.replace(new RegExp(testRootDir, 'g'), '/MOCK_ABOLUTE_PATH'),
test: val => typeof val === 'string' && val.includes(testRootDir),
Expand Down
6 changes: 6 additions & 0 deletions packages/jest-cli/src/cli/args.js
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,12 @@ const options = {
'adequately cleaned up.',
type: 'boolean',
},
generateCoverageForFiles: {
description:
'Run related tests for a list of source files that were passed in as ' +
'arguments and collect coverage for them',
type: 'boolean',
},
globals: {
description:
'A JSON string with map of global variables that need ' +
Expand Down
15 changes: 15 additions & 0 deletions packages/jest-config/src/normalize.js
Original file line number Diff line number Diff line change
Expand Up @@ -538,6 +538,21 @@ function normalize(options: InitialOptions, argv: Argv) {
.filter(reporter => reporter !== 'text');
}

// generateCoverageForFiles is an alias for
// `--findRelatedTests '/rootDir/file1.js' --coverage --collectCoverageFrom 'file1.js'`
// where arguments to `--collectCoverageFrom` should be globs (or relative
// paths to the rootDir)
if (argv.generateCoverageForFiles) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we good that it's only for CLI?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah! i don't think we'd want to put it in a config, since it's a very interactive option :)

newOptions.findRelatedTests = true;
newOptions.collectCoverage = true;
newOptions.collectCoverageFrom = argv._.map(filename => {
filename = _replaceRootDirInPath(options.rootDir, filename);
return path.isAbsolute(filename)
? path.relative(options.rootDir, filename)
: filename;
});
}

return {
hasDeprecationWarnings,
options: newOptions,
Expand Down
1 change: 1 addition & 0 deletions types/Argv.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export type Argv = {|
expand: boolean,
findRelatedTests: boolean,
forceExit: boolean,
generateCoverageForFiles: boolean,
globals: string,
h: boolean,
haste: string,
Expand Down