From 1328ff70cd69f02e48ec5505a1f93501b2bcaef4 Mon Sep 17 00:00:00 2001 From: Luna Ruan Date: Wed, 25 May 2022 17:53:44 -0700 Subject: [PATCH] [DevTools] Regression-proof e2e Tests (#24620) This PR: * Increases test retry count to 2 so that flaky tests have more of a chance to pass * Ideally most e2e tests will run for all React versions (and ensure DevTools elegantly fails if React doesn't support its features). However, some features aren't supported in older React versions at all (ex. Profiling) Add runOnlyForReactRange function in these cases to skip tests that don't satisfy the correct React semver range * Fix should allow searching for component by name test, which was flaky because sometimes the Searchbox would be unfocused the second time we try to type in it * Edited test Should allow elements to be inspected to check that element inspect gracefully fails in older React versions * Updated config to add a config.use.url field and a config.use.react_version field, which change depending on the React Version (and whether it's specified) --- .../__tests__/__e2e__/components.test.js | 86 +++++++++++++------ .../__tests__/__e2e__/profiler.test.js | 6 +- .../__tests__/__e2e__/utils.js | 17 ++++ .../playwright.config.js | 13 +++ .../src/devtools/views/Components/KeyValue.js | 6 +- 5 files changed, 99 insertions(+), 29 deletions(-) create mode 100644 packages/react-devtools-inline/__tests__/__e2e__/utils.js diff --git a/packages/react-devtools-inline/__tests__/__e2e__/components.test.js b/packages/react-devtools-inline/__tests__/__e2e__/components.test.js index 20a721beec..aaa201305b 100644 --- a/packages/react-devtools-inline/__tests__/__e2e__/components.test.js +++ b/packages/react-devtools-inline/__tests__/__e2e__/components.test.js @@ -2,10 +2,12 @@ 'use strict'; +const {runOnlyForReactRange} = require('./utils'); const listAppUtils = require('./list-app-utils'); const devToolsUtils = require('./devtools-utils'); const {test, expect} = require('@playwright/test'); const config = require('../../playwright.config'); +const semver = require('semver'); test.use(config); test.describe('Components', () => { let page; @@ -13,7 +15,7 @@ test.describe('Components', () => { test.beforeEach(async ({browser}) => { page = await browser.newPage(); - await page.goto('http://localhost:8080/e2e.html', { + await page.goto(config.use.url, { waitUntil: 'domcontentloaded', }); @@ -51,32 +53,60 @@ test.describe('Components', () => { // Select the first list item in DevTools. await devToolsUtils.selectElement(page, 'ListItem', 'List\nApp'); + // Prop names/values may not be editable based on the React version. + // If they're not editable, make sure they degrade gracefully + const isEditableName = semver.gte(config.use.react_version, '17.0.0'); + const isEditableValue = semver.gte(config.use.react_version, '16.8.0'); + // Then read the inspected values. - const [propName, propValue, sourceText] = await page.evaluate(() => { - const {createTestNameSelector, findAllNodes} = window.REACT_DOM_DEVTOOLS; - const container = document.getElementById('devtools'); + const [propName, propValue, sourceText] = await page.evaluate( + isEditable => { + const { + createTestNameSelector, + findAllNodes, + } = window.REACT_DOM_DEVTOOLS; + const container = document.getElementById('devtools'); - const editableName = findAllNodes(container, [ - createTestNameSelector('InspectedElementPropsTree'), - createTestNameSelector('EditableName'), - ])[0]; - const editableValue = findAllNodes(container, [ - createTestNameSelector('InspectedElementPropsTree'), - createTestNameSelector('EditableValue'), - ])[0]; - const source = findAllNodes(container, [ - createTestNameSelector('InspectedElementView-Source'), - ])[0]; + // Get name of first prop + const selectorName = isEditable.name + ? 'EditableName' + : 'NonEditableName'; + const nameElement = findAllNodes(container, [ + createTestNameSelector('InspectedElementPropsTree'), + createTestNameSelector(selectorName), + ])[0]; + const name = isEditable.name + ? nameElement.value + : nameElement.innerText; - return [editableName.value, editableValue.value, source.innerText]; - }); + // Get value of first prop + const selectorValue = isEditable.value + ? 'EditableValue' + : 'NonEditableValue'; + const valueElement = findAllNodes(container, [ + createTestNameSelector('InspectedElementPropsTree'), + createTestNameSelector(selectorValue), + ])[0]; + const source = findAllNodes(container, [ + createTestNameSelector('InspectedElementView-Source'), + ])[0]; + const value = isEditable.value + ? valueElement.value + : valueElement.innerText; + + return [name, value, source.innerText]; + }, + {name: isEditableName, value: isEditableValue} + ); expect(propName).toBe('label'); expect(propValue).toBe('"one"'); - expect(sourceText).toContain('ListApp.js'); + expect(sourceText).toMatch(/ListApp[a-zA-Z]*\.js/); }); test('should allow props to be edited', async () => { + runOnlyForReactRange('>=16.8'); + // Select the first list item in DevTools. await devToolsUtils.selectElement(page, 'ListItem', 'List\nApp'); @@ -109,6 +139,8 @@ test.describe('Components', () => { }); test('should load and parse hook names for the inspected element', async () => { + runOnlyForReactRange('>=16.8'); + // Select the List component DevTools. await devToolsUtils.selectElement(page, 'List', 'App'); @@ -162,19 +194,23 @@ test.describe('Components', () => { }); } - await page.evaluate(() => { - const {createTestNameSelector, focusWithin} = window.REACT_DOM_DEVTOOLS; - const container = document.getElementById('devtools'); + async function focusComponentSearch() { + await page.evaluate(() => { + const {createTestNameSelector, focusWithin} = window.REACT_DOM_DEVTOOLS; + const container = document.getElementById('devtools'); - focusWithin(container, [ - createTestNameSelector('ComponentSearchInput-Input'), - ]); - }); + focusWithin(container, [ + createTestNameSelector('ComponentSearchInput-Input'), + ]); + }); + } + await focusComponentSearch(); page.keyboard.insertText('List'); let count = await getComponentSearchResultsCount(); expect(count).toBe('1 | 4'); + await focusComponentSearch(); page.keyboard.insertText('Item'); count = await getComponentSearchResultsCount(); expect(count).toBe('1 | 3'); diff --git a/packages/react-devtools-inline/__tests__/__e2e__/profiler.test.js b/packages/react-devtools-inline/__tests__/__e2e__/profiler.test.js index 13612d4eb3..191c32fc7f 100644 --- a/packages/react-devtools-inline/__tests__/__e2e__/profiler.test.js +++ b/packages/react-devtools-inline/__tests__/__e2e__/profiler.test.js @@ -2,6 +2,7 @@ 'use strict'; +const {runOnlyForReactRange} = require('./utils'); const listAppUtils = require('./list-app-utils'); const devToolsUtils = require('./devtools-utils'); const {test, expect} = require('@playwright/test'); @@ -12,8 +13,7 @@ test.describe('Profiler', () => { test.beforeEach(async ({browser}) => { page = await browser.newPage(); - - await page.goto('http://localhost:8080/e2e.html', { + await page.goto(config.use.url, { waitUntil: 'domcontentloaded', }); @@ -23,6 +23,8 @@ test.describe('Profiler', () => { }); test('should record renders and commits when active', async () => { + // Profiling is only available in 16.5 and over + runOnlyForReactRange('>=16.5'); async function getSnapshotSelectorText() { return await page.evaluate(() => { const { diff --git a/packages/react-devtools-inline/__tests__/__e2e__/utils.js b/packages/react-devtools-inline/__tests__/__e2e__/utils.js new file mode 100644 index 0000000000..5f0505b71e --- /dev/null +++ b/packages/react-devtools-inline/__tests__/__e2e__/utils.js @@ -0,0 +1,17 @@ +'use strict'; + +/** @flow */ + +const semver = require('semver'); +const config = require('../../playwright.config'); +const {test} = require('@playwright/test'); + +function runOnlyForReactRange(range) { + test.skip( + !semver.satisfies(config.use.react_version, range), + `This test requires a React version of ${range} to run. ` + + `The React version you're using is ${config.use.react_version}` + ); +} + +module.exports = {runOnlyForReactRange}; diff --git a/packages/react-devtools-inline/playwright.config.js b/packages/react-devtools-inline/playwright.config.js index 8d65b94fd4..5e0f333bf2 100644 --- a/packages/react-devtools-inline/playwright.config.js +++ b/packages/react-devtools-inline/playwright.config.js @@ -1,3 +1,8 @@ +const semver = require('semver'); +const fs = require('fs'); +const ReactVersionSrc = fs.readFileSync(require.resolve('shared/ReactVersion')); +const reactVersion = /export default '([^']+)';/.exec(ReactVersionSrc)[1]; + const config = { use: { headless: true, @@ -7,7 +12,15 @@ const config = { // and DevTools operations to be sent across the bridge. slowMo: 100, }, + url: process.env.REACT_VERSION + ? 'http://localhost:8080/e2e-regression.html' + : 'http://localhost:8080/e2e.html', + react_version: process.env.REACT_VERSION + ? semver.coerce(process.env.REACT_VERSION).version + : reactVersion, }, + // Some of our e2e tests can be flaky. Retry tests to make sure the error isn't transient + retries: 2, }; module.exports = config; diff --git a/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.js b/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.js index b9fcf02145..5e805baf1f 100644 --- a/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.js +++ b/packages/react-devtools-shared/src/devtools/views/Components/KeyValue.js @@ -231,7 +231,7 @@ export default function KeyValue({ ); } else { renderedName = ( - + {name} {!!hookName && ({hookName})} @@ -286,7 +286,9 @@ export default function KeyValue({ {displayValue} ) : ( - {displayValue} + + {displayValue} + )} );