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

chore(webkit): add before:browser:launch and download support #23662

Merged
merged 35 commits into from
Sep 6, 2022

Conversation

flotwig
Copy link
Contributor

@flotwig flotwig commented Sep 1, 2022

User facing changelog

n/a

Additional details

  • Adds before:browser:launch support. launchOptions.extensions isn't supported since WK doesn't take extensions.
  • Adds support for config.downloadsFolder and download events.
  • Also fixes the issue where Cypress would reopen in interactive mode after cypress run ended, since it was making local system-tests fail.
  • Depends on chore(webkit): add video recording #23579

Steps to test

Refer to #23579 testing steps for instructions on installing WebKit.

  1. Run a project with before:browser:launch using WebKit, observe that the event is hit and that the launchOptions can be edited.
  2. Run a project that downloads files, note that they are saved to downloadsFolder when using WebKit.
  3. Exit a Cypress process after launching WebKit, note that Cypress no longer re-opens itself back up in open mode.

How has the user experience changed?

PR Tasks

  • Have tests been added/updated?
  • [na] Has the original issue (or this PR, if no issue exists) been tagged with a release in ZenHub? (user-facing changes only)
  • [na] Has a PR for user-facing changes been opened in cypress-documentation?
  • [na] Have API changes been updated in the type definitions?

@cypress-bot
Copy link
Contributor

cypress-bot bot commented Sep 1, 2022

Thanks for taking the time to open a PR!

@flotwig flotwig mentioned this pull request Sep 1, 2022
21 tasks
@cypress
Copy link

cypress bot commented Sep 1, 2022



Test summary

39674 0 3345 0Flakiness 1


Run details

Project cypress
Status Passed
Commit a58a915
Started Sep 6, 2022 5:24 PM
Ended Sep 6, 2022 5:39 PM
Duration 15:10 💡
OS Linux Debian - 11.3
Browser Multiple

View run in Cypress Dashboard ➡️


Flakiness

cypress/e2e/e2e/origin/commands/querying.cy.ts Flakiness
1 cy.origin querying > .get()

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@flotwig flotwig marked this pull request as ready for review September 2, 2022 15:01
* However, the Electron binary does not support an entrypoint, leading Cypress to think it's being opened in global mode (no args) when this fn is called.
* Solution is to filter out the problematic function.
* TODO(webkit): do we want to run this cleanup script another way?
* @see https://github.com/microsoft/playwright/blob/7e2aec7454f596af452b51a2866e86370291ac8b/packages/playwright-core/src/utils/processLauncher.ts#L191-L203
Copy link
Contributor

@lmiller1990 lmiller1990 Sep 6, 2022

Choose a reason for hiding this comment

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

Ahh this must have been tricky to debug. Nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Super tricky. The stdout/stderr isn't inherited and it doesn't pass --inspect-brk so it was like a ghost process. I eventually figured out what was going on by adding logging using fs.createWriteStream and tail'ing the file.

@lmiller1990
Copy link
Contributor

lmiller1990 commented Sep 6, 2022

I tried video recording. It worked!

I first tested a basic site using a JS based download. It works great (including video).

describe('empty spec', () => {
  it('passes', () => {
    cy.visit('http://localhost:8080')
    cy.get('input#btn').click()
    cy.readFile(`${Cypress.config('downloadsFolder')}/output.txt`)
  })
})

Website:

<!DOCTYPE html>
<html>

<head>
      <title>
            How to Download files Using JavaScript
      </title>
</head>

<body>

      <textarea id="text">DelftStack</textarea>
      <br />
      <input type="button" id="btn" value="Download" />
      <script>
            function download(filename, textInput) {

                  var element = document.createElement('a');
                  element.setAttribute('href','data:text/plain;charset=utf-8, ' + encodeURIComponent(textInput));
                  element.setAttribute('download', filename);
                  document.body.appendChild(element);
                  element.click();
                  //document.body.removeChild(element);
            }
            document.getElementById("btn")
                  .addEventListener("click", function () {
                        var text = document.getElementById("text").value;
                        var filename = "output.txt";
                        download(filename, text);
                  }, false);
      </script>
</body>

</html>

I also tried using a spammy test download site for my testing - it's more "real world".

Edit: this website exhibits the same weird problems in Cypress + Chrome - I think the problems are not WK specific.

describe('empty spec', () => {
  it('passes', () => {
    cy.visit('https://file-examples.com/index.php/sample-documents-download/sample-rtf-download/')
    cy.get('a').contains('Download sample rtf file').click()
  })
})

It did work in open mode - kind of - the file was downloaded. Due to the websites's jankiness, it somehow injected a Google Ad into the command log (wtf?). Also it shows an error:

image

It failed in run mode:

  empty spec
    1) passes


  0 passing (17s)
  1 failing

  1) empty spec
       passes:
     Error: The following error originated from your application code, not from Cypress.

  > Syntax error, unrecognized expression: https://file-examples.com/wp-content/uploads/2019/09/file-sample_100kB.rtf

When Cypress detects uncaught errors originating from your application it will automatically fail the current test.

This behavior is configurable, and you can choose to turn this off by listening to the `uncaught:exception` event.

https://on.cypress.io/uncaught-exception-from-application
  __CyWebKitError@
  @https://file-examples.com/wp-content/themes/file-examples/vendor/jquery/jquery.min.js:2:12742
  @https://file-examples.com/wp-content/themes/file-examples/vendor/jquery/jquery.min.js:2:18791
  @https://file-examples.com/wp-content/themes/file-examples/vendor/jquery/jquery.min.js:2:21595
  fa@https://file-examples.com/wp-content/themes/file-examples/vendor/jquery/jquery.min.js:2:7321
  find@https://file-examples.com/wp-content/themes/file-examples/vendor/jquery/jquery.min.js:2:24113
  @https://file-examples.com/wp-content/themes/file-examples/vendor/jquery/jquery.min.js:2:24680
  n@https://file-examples.com/wp-content/themes/file-examples/vendor/jquery/jquery.min.js:2:419
  @https://file-examples.com/wp-content/themes/file-examples/js/new-age.min.js:6:128
  dispatch@https://file-examples.com/wp-content/themes/file-examples/vendor/jquery/jquery.min.js:3:12449
  @https://file-examples.com/wp-content/themes/file-examples/vendor/jquery/jquery.min.js:3:9178
  dispatchEvent@[native code]
  sendEvent@
  _mouseClickEvents@
  click@
  onReady@
  onReady@
  retryActionability@
  tryCatcher@
  @
  tryCatcher@
  @
  @
  @
  @
  @
  @

@lmiller1990 lmiller1990 self-requested a review September 6, 2022 01:04
Copy link
Contributor

@lmiller1990 lmiller1990 left a comment

Choose a reason for hiding this comment

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

Code seems fine, I did some testing - see my comment.

Copy link
Contributor

@tbiethman tbiethman left a comment

Choose a reason for hiding this comment

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

Can confirm Cypress no longer relaunches after exiting in run mode.

Base automatically changed from webkit-video to develop September 6, 2022 17:12
@flotwig flotwig merged commit b5ba6d7 into develop Sep 6, 2022
@flotwig flotwig deleted the webkit-ux branch September 6, 2022 18:19
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