Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactored DevTools test shell for e2e #22968

Merged
merged 1 commit into from Dec 15, 2021

Conversation

bvaughn
Copy link
Contributor

@bvaughn bvaughn commented Dec 15, 2021

Fixes a regression in the e2e target and makes things easier (hopefully) going forward when adding new e2e tests.

I think we should follow up with a few things after this:

  • Harden the query selectors to avoid using things like "span[class^=Value]" (maybe the Test Selector API could help with this?)
  • Use something other than the ToDo app for our e2e tests (to avoid accidentally breaking them with changes to our test shell).

await page.goto('http://localhost:8080/', {waitUntil: 'domcontentloaded'});
await page.waitForSelector('iframe#target');
frameElementHandle = await page.$('#target');
await page.goto('http://localhost:8080/e2e.html', {waitUntil: 'domcontentloaded'});
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Longer term, relying on our test shell for e2e tests seems too fragile. Changing one shouldn't break the other. So I added a specific e2e target.

"start": "yarn start:app",
"start:app": "cross-env NODE_ENV=development cross-env TARGET=local webpack-dev-server --open-page app.html",
"start:multi": "cross-env NODE_ENV=development cross-env TARGET=local webpack-dev-server --open-page multi.html"
"start": "cross-env NODE_ENV=development cross-env TARGET=local webpack-dev-server"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I renamed the app.html file to index.html and added a small nav bar up top so we can get rid of these separate targets.

root.render(createElement(App));
}

mountApp(ToDoList);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might want to add e2e-specific things rather than reuse our ToDo app but this is okay for now.

@bvaughn bvaughn force-pushed the devtools-inline-e2e-tests branch 2 times, most recently from 6f4e76c to cf679b5 Compare December 15, 2021 15:41
@bvaughn
Copy link
Contributor Author

bvaughn commented Dec 15, 2021

Looks like NPM services are down at the moment so the Circle CI failures are not surprising (but also not caused by this PR).

@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Dec 15, 2021
This partially fixes a regression in the e2e target and makes things easier (hopefully) going forward when adding new e2e tests.
@bvaughn bvaughn merged commit aa8f2bd into facebook:main Dec 15, 2021
@bvaughn bvaughn deleted the devtools-inline-e2e-tests branch December 15, 2021 16:43
zhengjitf pushed a commit to zhengjitf/react that referenced this pull request Apr 15, 2022
Fixes a regression in the e2e target and makes things easier (hopefully) going forward when adding new e2e tests.
nevilm-lt pushed a commit to nevilm-lt/react that referenced this pull request Apr 22, 2022
Fixes a regression in the e2e target and makes things easier (hopefully) going forward when adding new e2e tests.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Component: Developer Tools React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants