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

Playwright build out #741

Merged
merged 1 commit into from
Sep 10, 2024
Merged

Playwright build out #741

merged 1 commit into from
Sep 10, 2024

Conversation

zerogravit1
Copy link
Collaborator

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.

Copy link
Collaborator

@benjaminpaige benjaminpaige left a 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.

Comment on lines 17 to 19
// expect(
await this.page.getByRole("link", { name: "Dashboard" }).isVisible();
// ).toBeTruthy();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Was this comment forgotten?

@@ -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}`);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This one too

Copy link
Collaborator

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

Copy link
Contributor

github-actions bot commented Aug 30, 2024

Coverage Report

Status Category Percentage Covered / Total
🔵 Lines 54.73% 4308 / 7870
🔵 Statements 54.57% 4537 / 8313
🔵 Functions 49.15% 1042 / 2120
🔵 Branches 27.51% 813 / 2955
File Coverage
File Stmts % Branch % Funcs % Lines Uncovered Lines
Changed Files
react-app/src/components/Layout/index.tsx 10.86% 0% 0% 11.62% 19-20, 22-24, 23, 27-50, 50, 52, 56, 58-60, 59, 62-64, 63, 66, 66, 120-122, 180-183, 185-192, 186, 188-191, 194-197, 195-196, 199, 199, 201-204, 202-208, 206-207, 210-251, 298, 298, 309-316
react-app/src/components/UsaBanner/index.tsx 77.77% 100% 71.42% 85.71% 77, 77
Generated in workflow #190

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
Copy link

codeclimate bot commented Sep 9, 2024

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.

Copy link
Contributor

🎉 This PR is included in version 1.5.0-val.79 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants