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

LPD-37310 Adjsut cookieBanner playwright test #4501

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

fortunatomaldonado
Copy link
Collaborator

https://liferay.atlassian.net/browse/LPD-37310

Trying to fix flaky playwright test.
Let me know if there should be any changes.

Thank you!

@liferay-continuous-integration
Copy link
Collaborator

CI is automatically triggering the following test suites:

  •     ci:test:sf

@liferay-continuous-integration
Copy link
Collaborator

✔️ ci:test:sf - 1 out of 1 jobs passed in 4 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: fafb2e6c9e66d62336eeeaa2298703536263e4ca

Sender Branch:

Branch Name: LPD-37310
Branch GIT ID: 8e7c656ffa05f7f643ff22c901b4f0dd195845ee

1 out of 1jobs PASSED
1 Successful Jobs:
For more details click here.

@liferay-continuous-integration
Copy link
Collaborator

@fortunatomaldonado
Copy link
Collaborator Author

ci:test:relevant

@liferay-continuous-integration
Copy link
Collaborator

Build completed.

Jenkins URL: test-portal-acceptance-pullrequest(master)

The relevant test suite did not execute tests because all modified files were excluded by the modified.files.global.excludes property in test.properties.

Please use ci:test:sf and/or ci:forward for this pull request to be forwarded or eligible for forwarding.

@liferay-continuous-integration
Copy link
Collaborator

Copy link
Collaborator

@markocikos markocikos left a comment

Choose a reason for hiding this comment

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

@fortunatomaldonado please see comments inline

@@ -52,7 +54,6 @@ test('@LPD-25701 Cookie Banner Script', async ({
await htmlExample.waitFor({state: 'visible'});
await htmlExample.click();
await htmlExample.click();
await htmlExample.click();
Copy link
Collaborator

Choose a reason for hiding this comment

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

You said in #4190 (comment) that triple click is the only way to edit. What changed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah this section is flaky as well and I'm thinking of redoing this a different way in order to prevent all the flaky tasks.

Comment on lines +122 to +125
await waitForAlert(
page,
`Success:Your request completed successfully.`
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this change is needed. waitForAlert defaults to this text.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought this too but was failing locally without it until I changed it. I'll revert it and see if I get any further errors.

await page.goto(layout.friendlyURL);
await page.goto(layout.friendlyUrlPath);

await page.locator('#content').waitFor({state: 'visible'});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of this, can we just await edit button?

const editButton = await page.getByRole('link', {name: 'Edit'});

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is also flaky it seems. It get stuck not finding the Edit button. Trying to avoid this.

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