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

feat(playwright): Clear cookie by name #4693

Merged

Conversation

ngraf
Copy link
Contributor

@ngraf ngraf commented Dec 30, 2024

Motivation/Description of the PR

  • new (since Playwright 1.43): Delete individual cookies with Playwright

Applicable helpers:

  • [ x ] Playwright
  • Puppeteer
  • WebDriver
  • REST
  • FileHelper
  • Appium
  • TestCafe

Type of change

  • 🔥 Breaking changes
  • [ x ] 🚀 New functionality
  • 🐛 Bug fix
  • 🧹 Chore
  • 📋 Documentation changes/updates
  • ♨️ Hot fix
  • 🔨 Markdown files fix - not related to source code
  • 💅 Polish code

Checklist:

  • [ x ] Tests have been added
  • Documentation has been added (Run npm run docs)
    -> npm run docs does not work for me :/
  • Lint checking (Run npm run lint)
    -> npm run lint does not work for me :/
  • Local tests are passed (Run npm test)
    -> npm test does not work for me :/

@DavertMik
Copy link
Contributor

DavertMik commented Dec 31, 2024

@ngraf PR looks good but tests failed. Could you check if it is needed to update Playwright in the pipeline?
https://github.com/codeceptjs/CodeceptJS/blob/3.x/.github/workflows/playwright.yml

@ngraf
Copy link
Contributor Author

ngraf commented Jan 2, 2025

Hi @DavertMik ,
I will take a look at the failing test.

At least I am able to reproduce the failing Playwright helper test locally now after following the steps in the GitHub pipeline.
I am surprised that in the test "I.grabCookie()" always returns nothing. I am a bit unsure if this is because the test setup, or a bug in the library.
I will investigate and keep this PR thread updated.

@ngraf
Copy link
Contributor Author

ngraf commented Jan 2, 2025

Alright. I fixed the tests.
I learned that I have to put an "await" in front of the "I" in the Playwright unit tests that rely on a real browser.

Now my final challenge is to resolve the branch conflict that arose in the meantime. That is a bit challenging for me, but I am confident I will resolve it .. somehow.

@ngraf
Copy link
Contributor Author

ngraf commented Jan 2, 2025

Alright. I resolved all conflicts.

I think we are good now. :)

@kobenguyent kobenguyent changed the title Feature: Clear cookie by name with Playwright feat(playwright): Clear cookie by name Jan 2, 2025
@@ -3,7 +3,7 @@ if none provided clears all cookies.

```js
I.clearCookie();
I.clearCookie('test'); // Playwright currently doesn't support clear a particular cookie name
I.clearCookie('test');
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe it's worth mentioning since Playwright 1.43 so that this won't surprise anyone when they are using playwright < 1.43 and this doesn't work for them.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's edge case, and I hope everyone will use latest PW

@DavertMik DavertMik merged commit 985b229 into codeceptjs:3.x Jan 2, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants