From 9011182c5539bb7e165b4e3430c545dce7706fa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Wed, 19 Jul 2017 10:35:45 -0700 Subject: [PATCH] Configure Jest with Stack and Fiber as separate projects (#10214) * Disable Fiber specific test run in CI This disables the comparison against previously recorded test. Instead, we'll rely on jest failures to fail tests. * Extract jest config into two separate projects for Fiber and Stack Allows us to run both in the same jest run. The setupMocks file is forked into specific environment configuration for each project. This replaces the environment variable. I used copy pasta here to make it clear. We can abstract this later. It's clear to me that simply extracting shared stuff is not the best way to abstract this. setupMocks for example didn't need all the code in both branches. I think that some of the stuff that is shared such as error message extracting etc. should probably be lifted out into a stand-alone jest project instead of being shared. * Fix class equivalence test There's a behavior change when projects are used which makes setupTestFrameworkScriptFile not override the normal config. This test should probably just move to a separate CI script or something less hacky. * Only run Fiber tests with scripts/fiber/record-tests --- package.json | 39 +---- scripts/circleci/test_entry_point.sh | 4 - scripts/circleci/test_fiber.sh | 9 -- scripts/fiber/record-tests | 7 +- scripts/jest/fiber.config.json | 38 +++++ .../jest/{setupMocks.js => fiber.setup.js} | 4 +- scripts/jest/setupSpecEquivalenceReporter.js | 2 - scripts/jest/stack.config.json | 38 +++++ scripts/jest/stack.setup.js | 20 +++ scripts/jest/test-framework-setup.js | 150 +++++++++--------- .../__tests__/ReactClassEquivalence-test.js | 18 +-- 11 files changed, 190 insertions(+), 139 deletions(-) delete mode 100755 scripts/circleci/test_fiber.sh create mode 100644 scripts/jest/fiber.config.json rename scripts/jest/{setupMocks.js => fiber.setup.js} (89%) create mode 100644 scripts/jest/stack.config.json create mode 100644 scripts/jest/stack.setup.js diff --git a/package.json b/package.json index 72da02d100..b255c106b2 100644 --- a/package.json +++ b/package.json @@ -120,40 +120,9 @@ "version-check": "node ./scripts/tasks/version-check.js" }, "jest": { - "modulePathIgnorePatterns": [ - "/.module-cache/", - "/build/", - "/scripts/rollup/shims/", - "/scripts/bench/" - ], - "rootDir": "", - "transform": { - ".*": "./scripts/jest/preprocessor.js" - }, - "setupFiles": [ - "./scripts/jest/environment.js" - ], - "setupTestFrameworkScriptFile": "./scripts/jest/test-framework-setup.js", - "testRegex": "/__tests__/.*(\\.js|coffee|ts)$", - "moduleFileExtensions": [ - "js", - "json", - "node", - "coffee", - "ts" - ], - "roots": [ - "/eslint-rules", - "/mocks", - "/scripts", - "/src", - "node_modules/fbjs" - ], - "collectCoverageFrom": [ - "src/**/*.js", - "!src/__mocks__/vendor/third_party/*.js", - "!src/test/*.js" - ], - "timers": "fake" + "projects": [ + "/scripts/jest/stack.config.json", + "/scripts/jest/fiber.config.json" + ] } } diff --git a/scripts/circleci/test_entry_point.sh b/scripts/circleci/test_entry_point.sh index 0000605c57..d42645c050 100755 --- a/scripts/circleci/test_entry_point.sh +++ b/scripts/circleci/test_entry_point.sh @@ -8,10 +8,6 @@ if [ $((1 % CIRCLE_NODE_TOTAL)) -eq "$CIRCLE_NODE_INDEX" ]; then COMMANDS_TO_RUN+=('./scripts/circleci/test_coverage.sh') fi -if [ $((2 % CIRCLE_NODE_TOTAL)) -eq "$CIRCLE_NODE_INDEX" ]; then - COMMANDS_TO_RUN+=('./scripts/circleci/test_fiber.sh') -fi - if [ $((3 % CIRCLE_NODE_TOTAL)) -eq "$CIRCLE_NODE_INDEX" ]; then COMMANDS_TO_RUN+=('node ./scripts/tasks/eslint') fi diff --git a/scripts/circleci/test_fiber.sh b/scripts/circleci/test_fiber.sh deleted file mode 100755 index 47fbf9cbe9..0000000000 --- a/scripts/circleci/test_fiber.sh +++ /dev/null @@ -1,9 +0,0 @@ -#!/bin/bash - -set -e - -echo 'Testing in fiber mode...' -./scripts/fiber/record-tests --track-facts --max-workers 1 -git --no-pager diff scripts/fiber -FIBER_TESTS_STATUS=$(git status --porcelain scripts/fiber) -test -z "$FIBER_TESTS_STATUS" diff --git a/scripts/fiber/record-tests b/scripts/fiber/record-tests index 6efa6c1b2a..668006f75a 100755 --- a/scripts/fiber/record-tests +++ b/scripts/fiber/record-tests @@ -15,8 +15,10 @@ const TestWatcher = require('jest').TestWatcher; const createContext = require('jest-runtime').createContext; const readConfig = require('jest-config').readConfig; -const argv = {}; const root = path.normalize(path.join(__dirname, '..', '..')); +const argv = { + config: path.join(root, 'scripts/jest/fiber.config.json'), +}; const testPathPattern = ''; function wrapRunnerFile(runnerPath) { @@ -79,7 +81,7 @@ function wrapRunner(originalPath) { } function runJest(maxWorkers) { - let { config } = readConfig(argv, root) + let { config } = readConfig(argv, root); config = Object.assign({}, config, { testRunner: wrapRunnerFile(config.testRunner), maxWorkers: maxWorkers, @@ -121,7 +123,6 @@ function formatResults(runResults, predicate) { } function recordTests(maxWorkers, trackFacts) { - process.env.REACT_JEST_USE_FIBER = true; runJest(maxWorkers) .then((runResults) => { const passing = formatResults( diff --git a/scripts/jest/fiber.config.json b/scripts/jest/fiber.config.json new file mode 100644 index 0000000000..c6f47bf86c --- /dev/null +++ b/scripts/jest/fiber.config.json @@ -0,0 +1,38 @@ +{ + "modulePathIgnorePatterns": [ + "/.module-cache/", + "/build/", + "/scripts/rollup/shims/", + "/scripts/bench/" + ], + "rootDir": "../../", + "transform": { + ".*": "./scripts/jest/preprocessor.js" + }, + "setupFiles": [ + "./scripts/jest/fiber.setup.js", + "./scripts/jest/environment.js" + ], + "setupTestFrameworkScriptFile": "./scripts/jest/test-framework-setup.js", + "testRegex": "/__tests__/.*(\\.js|coffee|ts)$", + "moduleFileExtensions": [ + "js", + "json", + "node", + "coffee", + "ts" + ], + "roots": [ + "/eslint-rules", + "/mocks", + "/scripts", + "/src", + "node_modules/fbjs" + ], + "collectCoverageFrom": [ + "src/**/*.js", + "!src/__mocks__/vendor/third_party/*.js", + "!src/test/*.js" + ], + "timers": "fake" +} diff --git a/scripts/jest/setupMocks.js b/scripts/jest/fiber.setup.js similarity index 89% rename from scripts/jest/setupMocks.js rename to scripts/jest/fiber.setup.js index c4743804b7..aadd3321bc 100644 --- a/scripts/jest/setupMocks.js +++ b/scripts/jest/fiber.setup.js @@ -5,7 +5,7 @@ jest.mock('ReactDOMFeatureFlags', () => { const flags = require.requireActual('ReactDOMFeatureFlags'); return Object.assign({}, flags, { - useFiber: !!process.env.REACT_JEST_USE_FIBER, + useFiber: true, }); }); jest.mock('ReactFeatureFlags', () => { @@ -17,7 +17,7 @@ jest.mock('ReactFeatureFlags', () => { jest.mock('ReactNativeFeatureFlags', () => { const flags = require.requireActual('ReactNativeFeatureFlags'); return Object.assign({}, flags, { - useFiber: !!process.env.REACT_JEST_USE_FIBER, + useFiber: true, }); }); diff --git a/scripts/jest/setupSpecEquivalenceReporter.js b/scripts/jest/setupSpecEquivalenceReporter.js index 9fa132d2a8..f432e3150d 100644 --- a/scripts/jest/setupSpecEquivalenceReporter.js +++ b/scripts/jest/setupSpecEquivalenceReporter.js @@ -9,8 +9,6 @@ 'use strict'; -require('./setupMocks'); - var expect = global.expect; var numExpectations = 0; diff --git a/scripts/jest/stack.config.json b/scripts/jest/stack.config.json new file mode 100644 index 0000000000..77972ca00c --- /dev/null +++ b/scripts/jest/stack.config.json @@ -0,0 +1,38 @@ +{ + "modulePathIgnorePatterns": [ + "/.module-cache/", + "/build/", + "/scripts/rollup/shims/", + "/scripts/bench/" + ], + "rootDir": "../../", + "transform": { + ".*": "./scripts/jest/preprocessor.js" + }, + "setupFiles": [ + "./scripts/jest/stack.setup.js", + "./scripts/jest/environment.js" + ], + "setupTestFrameworkScriptFile": "./scripts/jest/test-framework-setup.js", + "testRegex": "/__tests__/.*(\\.js|coffee|ts)$", + "moduleFileExtensions": [ + "js", + "json", + "node", + "coffee", + "ts" + ], + "roots": [ + "/eslint-rules", + "/mocks", + "/scripts", + "/src", + "node_modules/fbjs" + ], + "collectCoverageFrom": [ + "src/**/*.js", + "!src/__mocks__/vendor/third_party/*.js", + "!src/test/*.js" + ], + "timers": "fake" +} diff --git a/scripts/jest/stack.setup.js b/scripts/jest/stack.setup.js new file mode 100644 index 0000000000..f0ac8b26b3 --- /dev/null +++ b/scripts/jest/stack.setup.js @@ -0,0 +1,20 @@ +'use strict'; + +// We want to globally mock this but jest doesn't let us do that by default +// for a file that already exists. So we have to explicitly mock it. +jest.mock('ReactDOMFeatureFlags', () => { + const flags = require.requireActual('ReactDOMFeatureFlags'); + return Object.assign({}, flags, { + useFiber: false, + }); +}); +jest.mock('ReactNativeFeatureFlags', () => { + const flags = require.requireActual('ReactNativeFeatureFlags'); + return Object.assign({}, flags, { + useFiber: false, + }); +}); + +// Error logging varies between Fiber and Stack; +// Rather than fork dozens of tests, mock the error-logging file by default. +jest.mock('ReactFiberErrorLogger'); diff --git a/scripts/jest/test-framework-setup.js b/scripts/jest/test-framework-setup.js index 4c40e7b284..a77e03caa6 100644 --- a/scripts/jest/test-framework-setup.js +++ b/scripts/jest/test-framework-setup.js @@ -1,80 +1,84 @@ 'use strict'; -require('./setupMocks'); +if (process.env.REACT_CLASS_EQUIVALENCE_TEST) { + // Inside the class equivalence tester, we have a custom environment, let's + // require that instead. + require('./setupSpecEquivalenceReporter.js'); +} else { + var env = jasmine.getEnv(); -var env = jasmine.getEnv(); - -var callCount = 0; -var oldError = console.error; -var newError = function() { - callCount++; - oldError.apply(this, arguments); -}; - -console.error = newError; - -// TODO: Stop using spyOn in all the test since that seem deprecated. -// Legacy upgrade path from https://github.com/facebook/jest/blob/21a2b7aaee366af7ed87ae78c5b2d58cf3f5fb86/packages/jest-matchers/src/spy_matchers.js#L160 -const isSpy = spy => spy.calls && typeof spy.calls.count === 'function'; - -env.beforeEach(() => { - callCount = 0; - jasmine.addMatchers({ - toBeReset() { - return { - compare(actual) { - // TODO: Catch test cases that call spyOn() but don't inspect the mock - // properly. - if (actual !== newError && !isSpy(actual)) { - return { - pass: false, - message: () => - 'Test did not tear down console.error mock properly.', - }; - } - return {pass: true}; - }, - }; - }, - toNotHaveBeenCalled() { - return { - compare(actual) { - return { - pass: callCount === 0, - message: () => - 'Expected test not to warn. If the warning is expected, mock ' + - "it out using spyOn(console, 'error'); and test that the " + - 'warning occurs.', - }; - }, - }; - }, - }); -}); -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; - } + var callCount = 0; + var oldError = console.error; + var newError = function() { + callCount++; + oldError.apply(this, arguments); }; -} -const expectDev = function expectDev(actual) { - const expectation = expect(actual); - if (global.__suppressDevFailures) { - Object.keys(expectation).forEach(name => { - wrapDevMatcher(expectation, name); - wrapDevMatcher(expectation.not, name); + console.error = newError; + + // TODO: Stop using spyOn in all the test since that seem deprecated. + // Legacy upgrade path from https://github.com/facebook/jest/blob/21a2b7aaee366af7ed87ae78c5b2d58cf3f5fb86/packages/jest-matchers/src/spy_matchers.js#L160 + const isSpy = spy => spy.calls && typeof spy.calls.count === 'function'; + + env.beforeEach(() => { + callCount = 0; + jasmine.addMatchers({ + toBeReset() { + return { + compare(actual) { + // TODO: Catch test cases that call spyOn() but don't inspect the mock + // properly. + if (actual !== newError && !isSpy(actual)) { + return { + pass: false, + message: () => + 'Test did not tear down console.error mock properly.', + }; + } + return {pass: true}; + }, + }; + }, + toNotHaveBeenCalled() { + return { + compare(actual) { + return { + pass: callCount === 0, + message: () => + 'Expected test not to warn. If the warning is expected, mock ' + + "it out using spyOn(console, 'error'); and test that the " + + 'warning occurs.', + }; + }, + }; + }, }); + }); + 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; + } + }; } - return expectation; -}; -global.expectDev = expectDev; + + 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; +} diff --git a/src/isomorphic/modern/class/__tests__/ReactClassEquivalence-test.js b/src/isomorphic/modern/class/__tests__/ReactClassEquivalence-test.js index ea85c61667..8af26df7f2 100644 --- a/src/isomorphic/modern/class/__tests__/ReactClassEquivalence-test.js +++ b/src/isomorphic/modern/class/__tests__/ReactClassEquivalence-test.js @@ -32,16 +32,12 @@ function runJest(testFile) { var cwd = process.cwd(); var extension = process.platform === 'win32' ? '.cmd' : ''; var jestBin = path.resolve('node_modules', '.bin', 'jest' + extension); - var setupFile = path.resolve( - 'scripts', - 'jest', - 'setupSpecEquivalenceReporter.js', - ); - var result = spawnSync( - jestBin, - [testFile, '--setupTestFrameworkScriptFile', setupFile], - {cwd}, - ); + var result = spawnSync(jestBin, [testFile], { + cwd, + env: Object.assign({}, process.env, { + REACT_CLASS_EQUIVALENCE_TEST: 'true', + }), + }); if (result.error) { throw result.error; @@ -59,7 +55,7 @@ function runJest(testFile) { ); } - return result.stdout.toString(); + return result.stdout.toString() + result.stderr.toString(); } function compareResults(a, b) {