-
Notifications
You must be signed in to change notification settings - Fork 3
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
Playwright build out #741
Playwright build out #741
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I love this! especially the Page classses. It feels so much cleaner, and adding the dta tags.... 100% percent on board with that. This really looks like things are going well and good patterns are being tryed and established.
Before you go to far, there was some talk about Breaking out this app into a different app (one that would not live under the "react-app" directory. This has some benefits and tradeoffs but if you thik it might be woth diwscussing, lmk.
test/e2e/pages/loginPage.ts
Outdated
// expect( | ||
await this.page.getByRole("link", { name: "Dashboard" }).isVisible(); | ||
// ).toBeTruthy(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this comment forgotten?
test/e2e/playwright.config.ts
Outdated
@@ -18,7 +18,7 @@ const baseURL = process.env.STAGE_NAME | |||
).applicationEndpointUrl | |||
: "http://localhost:5000"; | |||
|
|||
console.log(`Playwright configured to run against ${baseURL}`); | |||
// console.log(`Playwright configured to run against ${baseURL}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one too
test/e2e/utils/auth.setup.ts
Outdated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the comments on this page as well
13ef22b
to
5c95703
Compare
Coverage Report
File Coverage
|
Add tests to validate banner Fix broken test Update tests - change reporter to dot, more CI friendly Resolve comments Resolve issues - remove used variable - updated selector to reduce flakiness
9fd3639
to
0156790
Compare
Code Climate has analyzed commit 0156790 and detected 0 issues on this pull request. The test coverage on the diff in this pull request is 100.0% (50% is the threshold). This pull request will bring the total coverage in the repository to 54.7% (0.0% change). View more on Code Climate. |
🎉 This PR is included in version 1.5.0-val.79 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Purpose
Describe the problem or feature in addition to a link to the issues.
Linked Issues to Close
Links to issue(s) that are closed by this PR. Be sure to use the phrase "Closes #XXX" for each issue, so they automatically close
Approach
How does this change address the issue?
Assorted Notes/Considerations/Learning
List any other information that you think is important... a post-merge activity, someone to notify, what you learned, etc.