Skip to content

Commit

Permalink
Reallowed single string arguments for pending tests
Browse files Browse the repository at this point in the history
- Continues to validate arguments to avoid accidental skips from jestjs#5558
  • Loading branch information
pk-nb committed Jul 2, 2018
1 parent d6e0a1b commit 922686d
Show file tree
Hide file tree
Showing 7 changed files with 55 additions and 82 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@

### Features

- `[jest-jasmine2]` Updated error throwing with `it`/ `test` to re-allow the valid single string argument for pending tests.
- `[jest-circus]` Updated error throwing with `it`/ `test` to re-allow the valid single string argument for pending tests
- `[jest-snapshot]` Introduce `toMatchInlineSnapshot` and `toThrowErrorMatchingInlineSnapshot` matchers ([#6380](https://github.com/facebook/jest/pull/6380))

### Chore & Maintenance
Expand Down
87 changes: 33 additions & 54 deletions e2e/__tests__/__snapshots__/globals.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -19,60 +19,6 @@ Time: <<REPLACED>>
Ran all test suites."
`;
exports[`cannot test with no implementation 1`] = `
"FAIL __tests__/only-constructs.test.js
● Test suite failed to run
Missing second argument. It must be a callback function.
1 |
2 | it('it', () => {});
> 3 | it('it, no implementation');
| ^
4 | test('test, no implementation');
5 |
at __tests__/only-constructs.test.js:3:5
"
`;
exports[`cannot test with no implementation 2`] = `
"Test Suites: 1 failed, 1 total
Tests: 0 total
Snapshots: 0 total
Time: <<REPLACED>>
Ran all test suites.
"
`;
exports[`cannot test with no implementation with expand arg 1`] = `
"FAIL __tests__/only-constructs.test.js
● Test suite failed to run
Missing second argument. It must be a callback function.
1 |
2 | it('it', () => {});
> 3 | it('it, no implementation');
| ^
4 | test('test, no implementation');
5 |
at __tests__/only-constructs.test.js:3:5
"
`;
exports[`cannot test with no implementation with expand arg 2`] = `
"Test Suites: 1 failed, 1 total
Tests: 0 total
Snapshots: 0 total
Time: <<REPLACED>>
Ran all test suites.
"
`;
exports[`function as descriptor 1`] = `
"PASS __tests__/function-as-descriptor.test.js
Foo
Expand Down Expand Up @@ -186,3 +132,36 @@ Snapshots: 0 total
Time: <<REPLACED>>
Ran all test suites."
`;
exports[`tests with no implementation 1`] = `
"PASS __tests__/only-constructs.test.js
✓ it
○ skipped 2 tests
"
`;
exports[`tests with no implementation 2`] = `
"Test Suites: 1 passed, 1 total
Tests: 2 skipped, 1 passed, 3 total
Snapshots: 0 total
Time: <<REPLACED>>
Ran all test suites."
`;
exports[`tests with no implementation with expand arg 1`] = `
"PASS __tests__/only-constructs.test.js
✓ it
○ it, no implementation
○ test, no implementation
"
`;
exports[`tests with no implementation with expand arg 2`] = `
"Test Suites: 1 passed, 1 total
Tests: 2 skipped, 1 passed, 3 total
Snapshots: 0 total
Time: <<REPLACED>>
Ran all test suites."
`;
8 changes: 4 additions & 4 deletions e2e/__tests__/globals.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ test('only', () => {
expect(summary).toMatchSnapshot();
});

test('cannot test with no implementation', () => {
test('tests with no implementation', () => {
const filename = 'only-constructs.test.js';
const content = `
it('it', () => {});
Expand All @@ -122,7 +122,7 @@ test('cannot test with no implementation', () => {

writeFiles(TEST_DIR, {[filename]: content});
const {stderr, status} = runJest(DIR);
expect(status).toBe(1);
expect(status).toBe(0);

const {summary} = extractSummary(stderr);
expect(cleanStderr(stderr)).toMatchSnapshot();
Expand Down Expand Up @@ -190,7 +190,7 @@ test('only with expand arg', () => {
expect(summary).toMatchSnapshot();
});

test('cannot test with no implementation with expand arg', () => {
test('tests with no implementation with expand arg', () => {
const filename = 'only-constructs.test.js';
const content = `
it('it', () => {});
Expand All @@ -200,7 +200,7 @@ test('cannot test with no implementation with expand arg', () => {

writeFiles(TEST_DIR, {[filename]: content});
const {stderr, status} = runJest(DIR, ['--expand']);
expect(status).toBe(1);
expect(status).toBe(0);

const {summary} = extractSummary(stderr);
expect(cleanStderr(stderr)).toMatchSnapshot();
Expand Down
20 changes: 10 additions & 10 deletions packages/jest-circus/src/__tests__/circus_it_test_error.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,21 @@ describe('test/it error throwing', () => {
circusIt('test1', () => {});
}).not.toThrowError();
});
it(`it throws error with missing callback function`, () => {
it(`it doesn't throw error with missing callback function`, () => {
expect(() => {
// $FlowFixMe: Easy, we're testing runitme errors here
// $FlowFixMe: Easy, we're testing runtime errors here
circusIt('test2');
}).toThrowError('Missing second argument. It must be a callback function.');
}).not.toThrowError();
});
it(`it throws an error when first argument isn't a string`, () => {
expect(() => {
// $FlowFixMe: Easy, we're testing runitme errors here
// $FlowFixMe: Easy, we're testing runtime errors here
circusIt(() => {});
}).toThrowError(`Invalid first argument, () => {}. It must be a string.`);
});
it('it throws an error when callback function is not a function', () => {
expect(() => {
// $FlowFixMe: Easy, we're testing runitme errors here
// $FlowFixMe: Easy, we're testing runtime errors here
circusIt('test4', 'test4b');
}).toThrowError(
`Invalid second argument, test4b. It must be a callback function.`,
Expand All @@ -58,21 +58,21 @@ describe('test/it error throwing', () => {
circusTest('test5', () => {});
}).not.toThrowError();
});
it(`test throws error with missing callback function`, () => {
it(`test doesn't throw error with missing callback function`, () => {
expect(() => {
// $FlowFixMe: Easy, we're testing runitme errors here
// $FlowFixMe: Easy, we're testing runtime errors here
circusTest('test6');
}).toThrowError('Missing second argument. It must be a callback function.');
}).not.toThrowError();
});
it(`test throws an error when first argument isn't a string`, () => {
expect(() => {
// $FlowFixMe: Easy, we're testing runitme errors here
// $FlowFixMe: Easy, we're testing runtime errors here
circusTest(() => {});
}).toThrowError(`Invalid first argument, () => {}. It must be a string.`);
});
it('test throws an error when callback function is not a function', () => {
expect(() => {
// $FlowFixMe: Easy, we're testing runitme errors here
// $FlowFixMe: Easy, we're testing runtime errors here
circusTest('test8', 'test8b');
}).toThrowError(
`Invalid second argument, test8b. It must be a callback function.`,
Expand Down
5 changes: 1 addition & 4 deletions packages/jest-circus/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,7 @@ const test = (testName: TestName, fn: TestFn, timeout?: number) => {
`Invalid first argument, ${testName}. It must be a string.`,
);
}
if (fn === undefined) {
throw new Error('Missing second argument. It must be a callback function.');
}
if (typeof fn !== 'function') {
if (fn !== undefined && typeof fn !== 'function') {
throw new Error(
`Invalid second argument, ${fn}. It must be a callback function.`,
);
Expand Down
8 changes: 4 additions & 4 deletions packages/jest-jasmine2/src/__tests__/it_test_error.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@
'use strict';

describe('test/it error throwing', () => {
it(`it throws error with missing callback function`, () => {
it(`it doesn't throw error with missing callback function`, () => {
expect(() => {
it('test1');
}).toThrowError('Missing second argument. It must be a callback function.');
}).not.toThrowError(/argument/i);
});
it(`it throws an error when first argument isn't a string`, () => {
expect(() => {
Expand All @@ -26,10 +26,10 @@ describe('test/it error throwing', () => {
`Invalid second argument, test3b. It must be a callback function.`,
);
});
test(`test throws error with missing callback function`, () => {
test(`test doesn't throw error with missing callback function`, () => {
expect(() => {
test('test4');
}).toThrowError('Missing second argument. It must be a callback function.');
}).not.toThrowError(/argument/i);
});
test(`test throws an error when first argument isn't a string`, () => {
expect(() => {
Expand Down
7 changes: 1 addition & 6 deletions packages/jest-jasmine2/src/jasmine/Env.js
Original file line number Diff line number Diff line change
Expand Up @@ -448,12 +448,7 @@ export default function(j$) {
`Invalid first argument, ${description}. It must be a string.`,
);
}
if (fn === undefined) {
throw new Error(
'Missing second argument. It must be a callback function.',
);
}
if (typeof fn !== 'function') {
if (fn !== undefined && typeof fn !== 'function') {
throw new Error(
`Invalid second argument, ${fn}. It must be a callback function.`,
);
Expand Down

0 comments on commit 922686d

Please sign in to comment.