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.

Sign up for GitHub

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

Jump to bottom

feat(testing): add playwright configuration generator #18013

Merged
merged 1 commit into from
Jul 14, 2023
Merged

feat(testing): add playwright configuration generator #18013

merged 1 commit into from
Jul 14, 2023

Conversation

xiongemi
Copy link
Collaborator

@xiongemi xiongemi commented Jul 7, 2023

Current Behavior

Expected Behavior

Related Issue(s)

Fixes #

@vercel
Copy link

vercel bot commented Jul 7, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 13, 2023 8:05pm

@nx-cloud
Copy link

nx-cloud bot commented Jul 7, 2023 •
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 13b7e4b. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch from 7eb8319 to 67218b1 Compare July 7, 2023 02:59
@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch from 67218b1 to 7bae525 Compare July 7, 2023 04:02
@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch from 7bae525 to 35938c7 Compare July 7, 2023 05:48
@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch from 35938c7 to 4700d53 Compare July 7, 2023 13:19
@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch from 4700d53 to 74fb52c Compare July 7, 2023 15:09
@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch from 74fb52c to 150b45d Compare July 7, 2023 15:13
@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch from 150b45d to e69e426 Compare July 7, 2023 16:43
barbados-clemens
barbados-clemens reviewed Jul 10, 2023
View reviewed changes
Copy link
Contributor

@barbados-clemens barbados-clemens left a comment

Choose a reason for hiding this comment

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

Just some initial thoughts.

packages/playwright/src/generators/configuration/generator.ts Outdated Show resolved Hide resolved
packages/playwright/src/generators/configuration/files/playwright.config.ts.template Outdated Show resolved Hide resolved
packages/playwright/src/generators/configuration/files/playwright.config.ts.template Outdated Show resolved Hide resolved
@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch from e69e426 to d6ca24d Compare July 12, 2023 16:56
@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch from d6ca24d to e033ecc Compare July 12, 2023 17:30
@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch from e033ecc to 367dce3 Compare July 12, 2023 17:46
@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch from 367dce3 to 50bc4a8 Compare July 12, 2023 18:00
@xiongemi xiongemi changed the title Feat/playwrite config generator feat(testing): add playwright configuration generator Jul 12, 2023
@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch from 50bc4a8 to bd6af9d Compare July 12, 2023 20:01
@xiongemi xiongemi requested review from a team as code owners July 12, 2023 21:10
@xiongemi xiongemi requested a review from AgentEnder July 12, 2023 21:10
barbados-clemens
barbados-clemens approved these changes Jul 12, 2023
View reviewed changes
Copy link
Contributor

@barbados-clemens barbados-clemens left a comment

Choose a reason for hiding this comment

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

other than not being able to get CI to pass,
everything looks good to me.

@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch from beb1cca to 608375a Compare July 12, 2023 21:43
@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch from 608375a to 68b7069 Compare July 13, 2023 15:22
@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch from 68b7069 to 345712e Compare July 13, 2023 16:23
mandarini
mandarini approved these changes Jul 13, 2023
View reviewed changes
Copy link
Member

@mandarini mandarini left a comment

Choose a reason for hiding this comment

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

Approved, but I added a comment, and also I have another minor comment:
for consistency maybe rename the packages/playwright/src/generators/configuration/generator.ts to packages/playwright/src/generators/configuration/configuration.ts and the packages/playwright/src/generators/init/generator.ts to packages/playwright/src/generators/init/init.ts, I think we tend to name the main generator file as the name of the generator/folder it is in! But not important!

.circleci/config.yml Outdated Show resolved Hide resolved
@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch from 345712e to 313b4f6 Compare July 13, 2023 18:35
@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch 2 times, most recently from 1d5b491 to fd8113a Compare July 13, 2023 18:57
@xiongemi
Copy link
Collaborator Author

Approved, but I added a comment, and also I have another minor comment: for consistency maybe rename the packages/playwright/src/generators/configuration/generator.ts to packages/playwright/src/generators/configuration/configuration.ts and the packages/playwright/src/generators/init/generator.ts to packages/playwright/src/generators/init/init.ts, I think we tend to name the main generator file as the name of the generator/folder it is in! But not important!

i renamed them. thanks

@xiongemi xiongemi force-pushed the feat/playwrite-config-generator branch from fd8113a to 13b7e4b Compare July 13, 2023 20:02
AgentEnder
AgentEnder approved these changes Jul 14, 2023
View reviewed changes
Copy link
Member

@AgentEnder AgentEnder left a comment

Choose a reason for hiding this comment

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

Looks good. Leaving 1 comment, feel free to address it or proceed as is.

packages/playwright/src/generators/configuration/configuration.ts Show resolved Hide resolved
@xiongemi xiongemi merged commit e5e561d into nrwl:master Jul 14, 2023
@github-actions
Copy link

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Moxita33 Moxita33 Moxita33 approved these changes

@barbados-clemens barbados-clemens barbados-clemens approved these changes

@AgentEnder AgentEnder AgentEnder approved these changes

@mandarini mandarini mandarini approved these changes

@jaysoo jaysoo jaysoo approved these changes

@meeroslav meeroslav Awaiting requested review from meeroslav meeroslav is a code owner

@vsavkin vsavkin Awaiting requested review from vsavkin vsavkin is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@xiongemi @jaysoo @mandarini @AgentEnder @barbados-clemens @Moxita33