Skip to content

Commit 115cd12

Browse files
acdliteacdlite
authored
Add test run that uses www feature flags (#18234)
In CI, we run our test suite against multiple build configurations. For example, we run our tests in both dev and prod, and in both the experimental and stable release channels. This is to prevent accidental deviations in behavior between the different builds. If there's an intentional deviation in behavior, the test author must account for them. However, we currently don't run tests against the www builds. That's a problem, because it's common for features to land in www before they land anywhere else, including the experimental release channel. Typically we do this so we can gradually roll out the feature behind a flag before deciding to enable it. The way we test those features today is by mutating the `shared/ReactFeatureFlags` module. There are a few downsides to this approach, though. The flag is only overridden for the specific tests or test suites where you apply the override. But usually what you want is to run *all* tests with the flag enabled, to protect against unexpected regressions. Also, mutating the feature flags module only works when running the tests against source, not against the final build artifacts, because the ReactFeatureFlags module is inlined by the build script. Instead, we should run the test suite against the www configuration, just like we do for prod, experimental, and so on. I've added a new command, `yarn test-www`. It automatically runs in CI. Some of the www feature flags are dynamic; that is, they depend on a runtime condition (i.e. a GK). These flags are imported from an external module that lives in www. Those flags will be enabled for some clients and disabled for others, so we should run the tests against *both* modes. So I've added a new global `__VARIANT__`, and a new test command `yarn test-www-variant`. `__VARIANT__` is set to false by default; when running `test-www-variant`, it's set to true. If we were going for *really* comprehensive coverage, we would run the tests against every possible configuration of feature flags: 2 ^ numberOfFlags total combinations. That's not practical, though, so instead we only run against two combinations: once with `__VARIANT__` set to `true`, and once with it set to `false`. We generally assume that flags can be toggled independently, so in practice this should be enough. You can also refer to `__VARIANT__` in tests to detect which mode you're running in. Or, you can import `shared/ReactFeatureFlags` and read the specific flag you can about. However, we should stop mutating that module going forward. Treat it as read-only. In this commit, I have only setup the www tests to run against source. I'll leave running against build for a follow up. Many of our tests currently assume they run only in the default configuration, and break when certain flags are toggled. Rather than fix these all up front, I've hard-coded the relevant flags to the default values. We can incrementally migrate those tests later.
1 parent 4027f2a commit 115cd12

File tree

12 files changed

+233
-9
lines changed

12 files changed

+233
-9
lines changed

‎.circleci/config.yml

Lines changed: 124 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,106 @@ jobs:
115115
RELEASE_CHANNEL: experimental
116116
command: yarn test --maxWorkers=2
117117

118+
test_source_www:
119+
docker: *docker
120+
environment: *environment
121+
steps:
122+
- checkout
123+
- *restore_yarn_cache
124+
- *run_yarn
125+
- run:
126+
environment:
127+
RELEASE_CHANNEL: stable
128+
command: yarn test-www --maxWorkers=2
129+
130+
test_source_www_variant:
131+
docker: *docker
132+
environment: *environment
133+
steps:
134+
- checkout
135+
- *restore_yarn_cache
136+
- *run_yarn
137+
- run:
138+
environment:
139+
RELEASE_CHANNEL: stable
140+
command: yarn test-www-variant --maxWorkers=2
141+
142+
test_source_www_prod:
143+
docker: *docker
144+
environment: *environment
145+
steps:
146+
- checkout
147+
- *restore_yarn_cache
148+
- *run_yarn
149+
- run:
150+
environment:
151+
NODE_ENV: production
152+
RELEASE_CHANNEL: stable
153+
command: yarn test-www --maxWorkers=2
154+
155+
test_source_www_variant_prod:
156+
docker: *docker
157+
environment: *environment
158+
steps:
159+
- checkout
160+
- *restore_yarn_cache
161+
- *run_yarn
162+
- run:
163+
environment:
164+
NODE_ENV: production
165+
RELEASE_CHANNEL: stable
166+
command: yarn test-www-variant --maxWorkers=2
167+
168+
test_source_www_experimental:
169+
docker: *docker
170+
environment: *environment
171+
steps:
172+
- checkout
173+
- *restore_yarn_cache
174+
- *run_yarn
175+
- run:
176+
environment:
177+
RELEASE_CHANNEL: experimental
178+
command: yarn test-www --maxWorkers=2
179+
180+
test_source_www_variant_experimental:
181+
docker: *docker
182+
environment: *environment
183+
steps:
184+
- checkout
185+
- *restore_yarn_cache
186+
- *run_yarn
187+
- run:
188+
environment:
189+
RELEASE_CHANNEL: experimental
190+
command: yarn test-www-variant --maxWorkers=2
191+
192+
test_source_www_prod_experimental:
193+
docker: *docker
194+
environment: *environment
195+
steps:
196+
- checkout
197+
- *restore_yarn_cache
198+
- *run_yarn
199+
- run:
200+
environment:
201+
NODE_ENV: production
202+
RELEASE_CHANNEL: experimental
203+
command: yarn test-www --maxWorkers=2
204+
205+
test_source_www_variant_prod_experimental:
206+
docker: *docker
207+
environment: *environment
208+
steps:
209+
- checkout
210+
- *restore_yarn_cache
211+
- *run_yarn
212+
- run:
213+
environment:
214+
NODE_ENV: production
215+
RELEASE_CHANNEL: experimental
216+
command: yarn test-www-variant --maxWorkers=2
217+
118218
test_source_persistent:
119219
docker: *docker
120220
environment: *environment
@@ -384,6 +484,18 @@ workflows:
384484
- test_source_persistent:
385485
requires:
386486
- setup
487+
- test_source_www:
488+
requires:
489+
- setup
490+
- test_source_www_variant:
491+
requires:
492+
- setup
493+
- test_source_www_prod:
494+
requires:
495+
- setup
496+
- test_source_www_variant_prod:
497+
requires:
498+
- setup
387499
- build:
388500
requires:
389501
- setup
@@ -415,6 +527,18 @@ workflows:
415527
- test_source_prod_experimental:
416528
requires:
417529
- setup
530+
- test_source_www_experimental:
531+
requires:
532+
- setup
533+
- test_source_www_variant_experimental:
534+
requires:
535+
- setup
536+
- test_source_www_prod_experimental:
537+
requires:
538+
- setup
539+
- test_source_www_variant_prod_experimental:
540+
requires:
541+
- setup
418542
- build_experimental:
419543
requires:
420544
- setup

‎.eslintrc.js

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,7 @@ const OFF = 0;
1111
const ERROR = 2;
1212

1313
module.exports = {
14-
extends: [
15-
'fbjs',
16-
'prettier'
17-
],
14+
extends: ['fbjs', 'prettier'],
1815

1916
// Stop ESLint from looking for a configuration file in parent folders
2017
root: true,
@@ -147,7 +144,7 @@ module.exports = {
147144
'scripts/**/*.js',
148145
'packages/*/npm/**/*.js',
149146
'packages/dom-event-testing-library/**/*.js',
150-
'packages/react-devtools*/**/*.js'
147+
'packages/react-devtools*/**/*.js',
151148
],
152149
rules: {
153150
'react-internal/no-production-logging': OFF,
@@ -171,6 +168,7 @@ module.exports = {
171168
__PROFILE__: true,
172169
__UMD__: true,
173170
__EXPERIMENTAL__: true,
171+
__VARIANT__: true,
174172
trustedTypes: true,
175173
},
176174
};

‎package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,8 @@
108108
"postinstall": "node node_modules/fbjs-scripts/node/check-dev-engines.js package.json && node ./scripts/flow/createFlowConfigs.js && node ./scripts/yarn/downloadReactIsForPrettyFormat.js",
109109
"debug-test": "cross-env NODE_ENV=development node --inspect-brk node_modules/jest/bin/jest.js --config ./scripts/jest/config.source.js --runInBand",
110110
"test": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source.js",
111+
"test-www": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source-www.js",
112+
"test-www-variant": "cross-env NODE_ENV=development VARIANT=true jest --config ./scripts/jest/config.source-www.js",
111113
"test-persistent": "cross-env NODE_ENV=development jest --config ./scripts/jest/config.source-persistent.js",
112114
"debug-test-persistent": "cross-env NODE_ENV=development node --inspect-brk node_modules/jest/bin/jest.js --config ./scripts/jest/config.source-persistent.js --runInBand",
113115
"test-prod": "cross-env NODE_ENV=production jest --config ./scripts/jest/config.source.js",

‎packages/react-dom/src/__tests__/ReactServerRendering-test.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ let React;
1414
let ReactDOMServer;
1515
let PropTypes;
1616
let ReactCurrentDispatcher;
17+
let enableSuspenseServerRenderer = require('shared/ReactFeatureFlags')
18+
.enableSuspenseServerRenderer;
1719

1820
function normalizeCodeLocInfo(str) {
1921
return str && str.replace(/\(at .+?:\d+\)/g, '(at **)');
@@ -686,7 +688,7 @@ describe('ReactDOMServer', () => {
686688
expect(markup).toBe('<div></div>');
687689
});
688690

689-
if (!__EXPERIMENTAL__) {
691+
if (!enableSuspenseServerRenderer) {
690692
it('throws for unsupported types on the server', () => {
691693
expect(() => {
692694
ReactDOMServer.renderToString(<React.Suspense />);

‎packages/react-reconciler/src/ReactFiber.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -973,10 +973,10 @@ export function assignFiberPropertiesInDEV(
973973
}
974974
if (enableUserTimingAPI) {
975975
target._debugID = source._debugID;
976+
target._debugIsCurrentlyTiming = source._debugIsCurrentlyTiming;
976977
}
977978
target._debugSource = source._debugSource;
978979
target._debugOwner = source._debugOwner;
979-
target._debugIsCurrentlyTiming = source._debugIsCurrentlyTiming;
980980
target._debugNeedsRemount = source._debugNeedsRemount;
981981
target._debugHookTypes = source._debugHookTypes;
982982
return target;

‎packages/shared/forks/ReactFeatureFlags.www-dynamic.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
/**
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*
7+
* @flow strict
8+
*/
9+
10+
// In www, these flags are controlled by GKs. Because most GKs have some
11+
// population running in either mode, we should run our tests that way, too,
12+
//
13+
// Use __VARIANT__ to simulate a GK. The tests will be run twice: once
14+
// with the __VARIANT__ set to `true`, and once set to `false`.
15+
16+
export const deferPassiveEffectCleanupDuringUnmount = __VARIANT__;
17+
export const runAllPassiveEffectDestroysBeforeCreates = __VARIANT__;
18+
export const warnAboutSpreadingKeyToJSX = __VARIANT__;
19+
20+
// These are already tested in both modes using the build type dimension,
21+
// so we don't need to use __VARIANT__ to get extra coverage.
22+
export const debugRenderPhaseSideEffectsForStrictMode = __DEV__;
23+
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = __DEV__;
24+
25+
// TODO: These flags are hard-coded to the default values used in open source.
26+
// Update the tests so that they pass in either mode, then set these
27+
// to __VARIANT__.
28+
export const enableTrustedTypesIntegration = false;
29+
export const warnAboutShorthandPropertyCollision = true;
30+
export const disableInputAttributeSyncing = false;
31+
export const disableSchedulerTimeoutBasedOnReactExpirationTime = false;
32+
export const enableModernEventSystem = false;

‎packages/shared/forks/ReactFeatureFlags.www.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,11 @@
99

1010
import typeof * as FeatureFlagsType from 'shared/ReactFeatureFlags';
1111
import typeof * as ExportsType from './ReactFeatureFlags.www';
12+
import typeof * as DynamicFeatureFlags from './ReactFeatureFlags.www-dynamic';
1213

1314
// Re-export dynamic flags from the www version.
15+
const dynamicFeatureFlags: DynamicFeatureFlags = require('ReactFeatureFlags');
16+
1417
export const {
1518
debugRenderPhaseSideEffectsForStrictMode,
1619
deferPassiveEffectCleanupDuringUnmount,
@@ -20,8 +23,9 @@ export const {
2023
warnAboutShorthandPropertyCollision,
2124
disableSchedulerTimeoutBasedOnReactExpirationTime,
2225
warnAboutSpreadingKeyToJSX,
26+
replayFailedUnitOfWorkWithInvokeGuardedCallback,
2327
enableModernEventSystem,
24-
} = require('ReactFeatureFlags');
28+
} = dynamicFeatureFlags;
2529

2630
// On WWW, __EXPERIMENTAL__ is used for a new modern build.
2731
// It's not used anywhere in production yet.
@@ -39,7 +43,6 @@ export const enableProfilerCommitHooks = false;
3943
export const enableSchedulerTracing = __PROFILE__;
4044
export const enableSchedulerDebugging = true;
4145

42-
export const replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
4346
export const warnAboutDeprecatedLifecycles = true;
4447
export const disableLegacyContext = __EXPERIMENTAL__;
4548
export const warnAboutStringRefs = false;

‎scripts/flow/environment.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
declare var __PROFILE__: boolean;
1313
declare var __UMD__: boolean;
1414
declare var __EXPERIMENTAL__: boolean;
15+
declare var __VARIANT__: boolean;
1516

1617
declare var __REACT_DEVTOOLS_GLOBAL_HOOK__: any; /*?{
1718
inject: ?((stuff: Object) => void)

‎scripts/jest/config.source-www.js

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
'use strict';
2+
3+
const baseConfig = require('./config.base');
4+
5+
const RELEASE_CHANNEL = process.env.RELEASE_CHANNEL;
6+
7+
// Default to building in experimental mode. If the release channel is set via
8+
// an environment variable, then check if it's "experimental".
9+
const __EXPERIMENTAL__ =
10+
typeof RELEASE_CHANNEL === 'string'
11+
? RELEASE_CHANNEL === 'experimental'
12+
: true;
13+
14+
const preferredExtension = __EXPERIMENTAL__ ? '.js' : '.stable.js';
15+
16+
const moduleNameMapper = {};
17+
moduleNameMapper[
18+
'^react$'
19+
] = `<rootDir>/packages/react/index${preferredExtension}`;
20+
moduleNameMapper[
21+
'^react-dom$'
22+
] = `<rootDir>/packages/react-dom/index${preferredExtension}`;
23+
24+
module.exports = Object.assign({}, baseConfig, {
25+
// Prefer the stable forks for tests.
26+
moduleNameMapper,
27+
modulePathIgnorePatterns: [
28+
...baseConfig.modulePathIgnorePatterns,
29+
'packages/react-devtools-shared',
30+
],
31+
setupFiles: [
32+
...baseConfig.setupFiles,
33+
require.resolve('./setupHostConfigs.js'),
34+
require.resolve('./setupTests.www.js'),
35+
],
36+
});

‎scripts/jest/setupEnvironment.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,8 @@ global.__EXPERIMENTAL__ =
1717
? RELEASE_CHANNEL === 'experimental'
1818
: true;
1919

20+
global.__VARIANT__ = !!process.env.VARIANT;
21+
2022
if (typeof window !== 'undefined') {
2123
global.requestIdleCallback = function(callback) {
2224
return setTimeout(() => {

0 commit comments

Comments
 (0)