Skip to content

Commit

Permalink
Build infra for tracking dev-specific failures (facebook#8228)
Browse files Browse the repository at this point in the history
I'll plan to change all of our console.error and component-tree expects to expectDev. It's a little annoying that we need to make sure tests don't throw (see my change to normalizeCodeLocInfo) but any alternative would seem to require two separate test runs or a much more cumbersome syntax.
  • Loading branch information
sophiebits authored Nov 13, 2016
1 parent c6f10e2 commit 64cba04
Show file tree
Hide file tree
Showing 7 changed files with 87 additions and 18 deletions.
12 changes: 9 additions & 3 deletions .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
'use strict';

const OFF = 0;
const WARNING = 1;
// const WARNING = 1;
const ERROR = 2;

module.exports = {
Expand All @@ -13,7 +15,7 @@ module.exports = {
],

ecmaFeatures: {
modules: false
modules: false,
},

// We're stricter than the default config, mostly. We'll override a few rules
Expand Down Expand Up @@ -70,5 +72,9 @@ module.exports = {
// CUSTOM RULES
// the second argument of warning/invariant should be a literal string
'react-internal/warning-and-invariant-args': ERROR,
}
},

globals: {
expectDev: true,
},
};
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
"gzip-js": "~0.3.2",
"jest": "^15.1.1",
"jest-config": "^15.1.1",
"jest-jasmine2": "^15.1.1",
"jest-runtime": "^15.1.1",
"loose-envify": "^1.1.0",
"merge-stream": "^1.0.0",
Expand Down
52 changes: 46 additions & 6 deletions scripts/fiber/record-tests
Original file line number Diff line number Diff line change
Expand Up @@ -27,19 +27,51 @@ function wrapRunnerFile(runnerPath) {
filename,
`
'use strict';
var runnerPath = ${JSON.stringify(runnerPath)};
var wrap = require(${JSON.stringify(__filename)}).wrapRunner;
var original = require(${JSON.stringify(runnerPath)});
module.exports = wrap(original);
module.exports = wrap(runnerPath);
`
);
return filename;
}

function wrapRunner(original) {
return function runner(config, environment, runtime, testPath) {
return original(config, environment, runtime, testPath)
function wrapRunner(originalPath) {
const original = require(originalPath);
// Assuming originalPath is .../jest-jasmine2/build/index.js
const JasmineReporter = require(
path.join(path.dirname(originalPath), 'reporter.js')
);

// For each spec, we store whether there was any expectDev() failure. This
// relies on the results being returned in the same order as they are run.
const hadDevFailures = [];
let environment;

const oldSpecStarted = JasmineReporter.prototype.specStarted;
JasmineReporter.prototype.specStarted = function(result) {
oldSpecStarted.apply(this, arguments);

environment.global.__suppressDevFailures = true;
environment.global.__hadDevFailures = false;
};

const oldSpecDone = JasmineReporter.prototype.specDone;
JasmineReporter.prototype.specDone = function(result) {
oldSpecDone.apply(this, arguments);

environment.global.__suppressDevFailures = false;
hadDevFailures.push(environment.global.__hadDevFailures);
};

return function runner(config, env, runtime, testPath) {
environment = env;
return original(config, env, runtime, testPath)
.then((results) => {
results.failureMessage = null;
hadDevFailures.forEach((hadFailures, i) => {
results.testResults[i].hadDevFailures = hadFailures;
});
hadDevFailures.length = 0;
return results;
});
};
Expand Down Expand Up @@ -91,7 +123,11 @@ function recordTests(trackFacts) {
.then((runResults) => {
const passing = formatResults(
runResults,
(file, test) => test.status === 'passed'
(file, test) => test.status === 'passed' && !test.hadDevFailures
);
const passingExceptDev = formatResults(
runResults,
(file, test) => test.status === 'passed' && test.hadDevFailures
);
const failing = formatResults(
runResults,
Expand All @@ -101,6 +137,10 @@ function recordTests(trackFacts) {
path.join(__dirname, 'tests-passing.txt'),
passing + '\n'
);
fs.writeFileSync(
path.join(__dirname, 'tests-passing-except-dev.txt'),
passingExceptDev + '\n'
);
fs.writeFileSync(
path.join(__dirname, 'tests-failing.txt'),
failing + '\n'
Expand Down
4 changes: 0 additions & 4 deletions scripts/fiber/tests-failing.txt
Original file line number Diff line number Diff line change
Expand Up @@ -505,10 +505,6 @@ src/renderers/shared/shared/__tests__/ReactTreeTraversal-test.js
* should leave to the window
* should leave to the window from the shallowest

src/renderers/shared/stack/reconciler/__tests__/ReactChildReconciler-test.js
* warns for duplicated keys
* warns for duplicated keys with component stack info

src/renderers/shared/stack/reconciler/__tests__/ReactComponent-test.js
* should throw on invalid render targets
* should throw when supplying a ref outside of render method
Expand Down
3 changes: 3 additions & 0 deletions scripts/fiber/tests-passing-except-dev.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
src/renderers/shared/stack/reconciler/__tests__/ReactChildReconciler-test.js
* warns for duplicated keys
* warns for duplicated keys with component stack info
23 changes: 23 additions & 0 deletions scripts/jest/test-framework-setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,3 +58,26 @@ env.afterEach(() => {
expect(console.error).toBeReset();
expect(console.error).toNotHaveBeenCalled();
});

function wrapDevMatcher(obj, name) {
const original = obj[name];
obj[name] = function devMatcher() {
try {
original.apply(this, arguments);
} catch (e) {
global.__hadDevFailures = e.stack;
}
};
}

const expectDev = function expectDev(actual) {
const expectation = expect(actual);
if (global.__suppressDevFailures) {
Object.keys(expectation).forEach((name) => {
wrapDevMatcher(expectation, name);
wrapDevMatcher(expectation.not, name);
});
}
return expectation;
};
global.expectDev = expectDev;
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ var ReactTestUtils;

describe('ReactChildReconciler', () => {
function normalizeCodeLocInfo(str) {
return str.replace(/\(at .+?:\d+\)/g, '(at **)');
return str && str.replace(/\(at .+?:\d+\)/g, '(at **)');
}

beforeEach(() => {
Expand All @@ -40,8 +40,8 @@ describe('ReactChildReconciler', () => {

ReactTestUtils.renderIntoDocument(<Component />);

expect(console.error.calls.count()).toBe(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
expectDev(console.error.calls.count()).toBe(1);
expectDev(console.error.calls.argsFor(0)[0]).toContain(
'Child keys must be unique; when two children share a key, only the first child will be used.'
);
});
Expand Down Expand Up @@ -69,8 +69,8 @@ describe('ReactChildReconciler', () => {

ReactTestUtils.renderIntoDocument(<GrandParent />);

expect(console.error.calls.count()).toBe(1);
expect(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
expectDev(console.error.calls.count()).toBe(1);
expectDev(normalizeCodeLocInfo(console.error.calls.argsFor(0)[0])).toBe(
'Warning: flattenChildren(...): ' +
'Encountered two children with the same key, `1`. ' +
'Child keys must be unique; when two children share a key, ' +
Expand Down

0 comments on commit 64cba04

Please sign in to comment.