test(playwright): Convert Cypress tests to Playwright - 2 #1246

xrutayisire · 2023-12-15T12:16:13Z

Context

Completes DT-1856

The Solution

Sorry for the big PR 😬

This PR include:

Allow parallel tests
Remove any reference to login since we don't need it anymore (yet)
Run tests on prod version not dev to speed up execution
Improve mock procedures function to allow mocking multiple times the same procedure
Comment Cypress CI since it's crashing without alpha now
Remove converted Cypress tests
Rename "Got It" to "Got it" where it's not done
Improve README best practices
Handle mock with functions to generate more than one
Create tests for Review dialog
Creates tests for more push use cases
Creates tests for update available in side menu bar
Create tests for tutorial video tooltip

Impact / Dependencies

None

Checklist before requesting a review

I hereby declare my code ready for review.
If it is a critical feature, I have added tests.
The CI is successful.
If there could backward compatibility issues, it has been discussed and planned.

linear · 2023-12-15T12:16:15Z

DT-1856 [e2e][test] Convert Cypress test to Playwright - 2

vercel · 2023-12-15T12:16:18Z

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

Name	Status	Preview	Updated (UTC)
slice-machine	✅ Ready (Inspect)	Visit Preview	Dec 27, 2023 11:45am

angeloashmore

Looks good to me! I just had a few questions I left as comments throughout the code.

Could you explain how we are able to remove the loggedIn test config? Should we be mocking manager responses for anything that requires authentication?

angeloashmore · 2023-12-27T01:46:29Z

playwright/pages/components/ReviewDialog.ts

+    /**
+     * Static locators
+     */
+    this.messageTextarea = this.dialog.getByPlaceholder("Tell us more...", {


Do we have a label to use instead of placeholder? If we do, could we use getByLabel() instead of getByPlaceholder()?

If not, it's something we may want to raise as an accessibility issue.

Sadly there is no label and that's not great. It's an old component and when we will redo it we need to change it.

angeloashmore · 2023-12-27T01:48:27Z

playwright/mocks/generateLibraries.ts

+
+import { generateRandomId } from "../utils";
+
+type GenerateTypesArgs = { nbSlices: number };


What does nbSlices mean? "nb" is unclear - is there a clearer name?

Yes sorry my bad here, slicesCount is better!

angeloashmore · 2023-12-27T01:48:50Z

playwright/mocks/generateTypes.ts

+import { generateRandomId } from "../utils";
+
+type GenerateTypesArgs = {
+  nbTypes: number;


Same comment as above: is there a clearer name than "nb"?

typesCount, thanks, you're right

angeloashmore · 2023-12-27T01:54:02Z

playwright/tests/common/sideNav.spec.ts

+import { test } from "../../fixtures";
+import { generateLibraries, generateTypes } from "../../mocks";
+
+test.describe("Side nav", () => {


This isn't a comment specific to this PR: Do you think the describe groups are necessary?

We seem to have one describe per .spec.ts file. The Playwright UI list could be shallower if we removed the describe groups.

YESSSS! I had the same thought and I want to remove them ALL!

Because we prefer to have a small file, and so create another file instead of another describe, we can and should remove them.
I will do it right after the two PRs, or just before merging the second one so I don't have conflict with myself 😅

xrutayisire · 2023-12-27T10:46:29Z

@angeloashmore

Could you explain how we are able to remove the loggedIn test config? Should we be mocking manager responses for anything that requires authentication?

Yes, for standalone test at the moment. It finally set a common ground for all standalone tests that we are writing now.

We mock everything that query the outside world that we want to test for a specific test. So if you test something related to an authentication in wroom, yes mock.

DX is much nicer like that since you don't care about anything external, email, password, etc... Let me know what you think and we could come back to something different.

xrutayisire changed the base branch from master to dev-next-release December 15, 2023 12:19

vercel bot deployed to Preview December 15, 2023 12:28 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from c17848d to a98fb39 Compare December 15, 2023 14:40

xrutayisire changed the title ~~test(playwright): Convert Cypress tests to Playwright 2~~ test(playwright): Convert Cypress tests to Playwright - 2 Dec 15, 2023

vercel bot deployed to Preview December 15, 2023 14:58 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from a98fb39 to 7b913de Compare December 15, 2023 15:00

vercel bot deployed to Preview December 15, 2023 15:10 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from 7b913de to 1d4b620 Compare December 15, 2023 15:28

vercel bot deployed to Preview December 15, 2023 15:35 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from 1d4b620 to a2a4a6c Compare December 15, 2023 16:01

vercel bot deployed to Preview December 15, 2023 16:07 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from a2a4a6c to e852adf Compare December 15, 2023 17:39

vercel bot deployed to Preview December 15, 2023 17:44 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from e852adf to 099feb3 Compare December 15, 2023 18:05

vercel bot deployed to Preview December 15, 2023 18:10 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from 099feb3 to dc9a343 Compare December 16, 2023 16:43

vercel bot deployed to Preview December 16, 2023 16:48 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from dc9a343 to 10ca9af Compare December 17, 2023 00:20

vercel bot deployed to Preview December 17, 2023 00:26 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from 10ca9af to 20bfcfd Compare December 17, 2023 00:43

vercel bot deployed to Preview December 17, 2023 00:49 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from 20bfcfd to c0e623c Compare December 17, 2023 05:05

vercel bot deployed to Preview December 17, 2023 05:11 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from c0e623c to dfced22 Compare December 17, 2023 05:32

vercel bot deployed to Preview December 17, 2023 05:38 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from dfced22 to 8e69332 Compare December 17, 2023 13:34

vercel bot deployed to Preview December 17, 2023 13:39 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from 8e69332 to 1daa3bd Compare December 17, 2023 13:53

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from 65dd763 to 9db8cb4 Compare December 23, 2023 00:01

vercel bot deployed to Preview December 23, 2023 00:06 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from 9db8cb4 to c19c699 Compare December 23, 2023 00:18

xrutayisire marked this pull request as draft December 23, 2023 00:18

vercel bot deployed to Preview December 23, 2023 00:23 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from c19c699 to 0ecd40d Compare December 23, 2023 00:28

vercel bot deployed to Preview December 23, 2023 00:33 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from 0ecd40d to f2cc98b Compare December 23, 2023 00:39

vercel bot deployed to Preview December 23, 2023 00:44 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from f2cc98b to 1d2f9f5 Compare December 23, 2023 00:49

vercel bot deployed to Preview December 23, 2023 00:54 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from 1d2f9f5 to 0eb97bd Compare December 23, 2023 00:58

vercel bot deployed to Preview December 23, 2023 01:04 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from 0eb97bd to 6397b02 Compare December 26, 2023 22:12

vercel bot deployed to Preview December 26, 2023 22:17 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from 6397b02 to 94e6306 Compare December 26, 2023 22:24

vercel bot deployed to Preview December 26, 2023 22:33 View deployment

xrutayisire marked this pull request as ready for review December 26, 2023 22:55

angeloashmore approved these changes Dec 27, 2023

View reviewed changes

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch 2 times, most recently from 198cf40 to dabecf2 Compare December 27, 2023 11:18

vercel bot deployed to Preview December 27, 2023 11:24 View deployment

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from dabecf2 to 3126c5d Compare December 27, 2023 11:33

test(playwright): Convert Cypress tests to Playwright - 2

56cf806

xrutayisire force-pushed the xru/convert-cypress-tests-2 branch from 3126c5d to 56cf806 Compare December 27, 2023 11:39

vercel bot deployed to Preview December 27, 2023 11:45 View deployment

xrutayisire merged commit 53fe5b3 into main Dec 27, 2023
33 checks passed

xrutayisire deleted the xru/convert-cypress-tests-2 branch December 27, 2023 12:10

xrutayisire mentioned this pull request Dec 27, 2023

test(playwright): Remove describe in test files #1256

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

test(playwright): Convert Cypress tests to Playwright - 2 #1246

test(playwright): Convert Cypress tests to Playwright - 2 #1246

xrutayisire commented Dec 15, 2023 •

edited

Loading

linear bot commented Dec 15, 2023

vercel bot commented Dec 15, 2023 •

edited

Loading

angeloashmore left a comment

angeloashmore Dec 27, 2023

xrutayisire Dec 27, 2023 •

edited

Loading

angeloashmore Dec 27, 2023

xrutayisire Dec 27, 2023

angeloashmore Dec 27, 2023

xrutayisire Dec 27, 2023

angeloashmore Dec 27, 2023

xrutayisire Dec 27, 2023 •

edited

Loading

xrutayisire commented Dec 27, 2023


		import { generateRandomId } from "../utils";

		type GenerateTypesArgs = { nbSlices: number };

test(playwright): Convert Cypress tests to Playwright - 2 #1246

test(playwright): Convert Cypress tests to Playwright - 2 #1246

Conversation

xrutayisire commented Dec 15, 2023 • edited Loading

Context

The Solution

Impact / Dependencies

Checklist before requesting a review

linear bot commented Dec 15, 2023

vercel bot commented Dec 15, 2023 • edited Loading

angeloashmore left a comment

Choose a reason for hiding this comment

angeloashmore Dec 27, 2023

Choose a reason for hiding this comment

xrutayisire Dec 27, 2023 • edited Loading

Choose a reason for hiding this comment

angeloashmore Dec 27, 2023

Choose a reason for hiding this comment

xrutayisire Dec 27, 2023

Choose a reason for hiding this comment

angeloashmore Dec 27, 2023

Choose a reason for hiding this comment

xrutayisire Dec 27, 2023

Choose a reason for hiding this comment

angeloashmore Dec 27, 2023

Choose a reason for hiding this comment

xrutayisire Dec 27, 2023 • edited Loading

Choose a reason for hiding this comment

xrutayisire commented Dec 27, 2023

xrutayisire commented Dec 15, 2023 •

edited

Loading

vercel bot commented Dec 15, 2023 •

edited

Loading

xrutayisire Dec 27, 2023 •

edited

Loading

xrutayisire Dec 27, 2023 •

edited

Loading