Skip to content

Adding code to fix force click and ignore HTTPS errors in playwright #2566

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

Merged
merged 3 commits into from
Sep 17, 2020
Merged

Adding code to fix force click and ignore HTTPS errors in playwright #2566

merged 3 commits into from
Sep 17, 2020

Conversation

gurjeetbains
Copy link
Contributor

@gurjeetbains gurjeetbains commented Aug 31, 2020

Motivation/Description of the PR

I am exploring playwright as an option in my organization and SSL certificate errors plus forceClick fix was one of the functions that we needed to fix so that we could proceed further

Applicable helpers:

  • WebDriver
  • Puppeteer
  • Nightmare
  • REST
  • FileHelper
  • Appium
  • Protractor
  • TestCafe
  • [-] Playwright

Applicable plugins:

  • allure
  • autoDelay
  • autoLogin
  • customLocator
  • pauseOnFail
  • puppeteerCoverage
  • retryFailedStep
  • screenshotOnFail
  • selenoid
  • stepByStepReport
  • wdio

Type of change

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

Checklist:

  • Tests have been added (Not applicable)
  • Documentation has been added (Run npm run docs) (Don't know about this but I added comments)
  • Lint checking (Run npm run lint)
  • [-] Local tests are passed (Run npm test) Yes they passed only for file system it failed but the contents seemed similar

Copy link
Contributor

@DavertMik DavertMik left a comment

Choose a reason for hiding this comment

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

Fine with me, just have a concern about default values

There was type here rectified it and it should be finw
Copy link
Contributor

@DavertMik DavertMik left a comment

Choose a reason for hiding this comment

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

Our CI says that tests are failing for this PR.
Tests are failing on Playwright - so this is related to this PR.
There are 2 options, you either fix tests by yourself (we have contributing guide with details on tests) or you wait for me - I will work on the next release and this PR later this week.

@@ -542,7 +543,7 @@ class Playwright extends Helper {
this.browser.on('targetchanged', (target) => {
this.debugSection('Url', target.url());
});
this.browserContext = await this.browser.newContext({ acceptDownloads: true, ...this.options.emulate });
this.browserContext = await this.browser.newContext({ ignoreHTTPSErrors :this.options.ignoreHTTPSErrors,acceptDownloads: true, ...this.options.emulate });//Adding the HTTPSError ignore in the context so that we can ignore those errors
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
this.browserContext = await this.browser.newContext({ ignoreHTTPSErrors :this.options.ignoreHTTPSErrors,acceptDownloads: true, ...this.options.emulate });//Adding the HTTPSError ignore in the context so that we can ignore those errors
this.browserContext = await this.browser.newContext({ ignoreHTTPSErrors: this.options.ignoreHTTPSErrors,acceptDownloads: true, ...this.options.emulate });//Adding the HTTPSError ignore in the context so that we can ignore those errors

Copy link
Contributor

Choose a reason for hiding this comment

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

please fix formatting issues like this one - npm run lint-fix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it for me ??

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@DavertMik I have committed the change requested

Adding this suggestion as it fixed the formatting

Co-authored-by: Michael Bodnarchuk <DavertMik@users.noreply.github.com>
@DavertMik
Copy link
Contributor

Thank you!
I will try to pick up from here and fix failing tests

@DavertMik DavertMik merged commit b2affe8 into codeceptjs:master Sep 17, 2020
@lepapareil
Copy link

Hi guys,

I think the page https://codecept.io/helpers/Playwright/#configuration is not up to date because the new ignoreHTTPSErrors option is missing

@mirao
Copy link
Contributor

mirao commented Oct 13, 2022

@lepapareil I've created #3456, it covers also docs.

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.

5 participants