8000 Migrate testing infrastructure on 3.x-stable by timmywil · Pull Request #5427 · jquery/jquery · GitHub
[go: up one dir, main page]

Skip to content

Migrate testing infrastructure on 3.x-stable #5427

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 8000 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Mar 5, 2024

Conversation

timmywil
Copy link
Member
@timmywil timmywil commented Mar 2, 2024

Summary

This is the 3.x version of #5418. There are some changes in this PR that I plan on copying to the main branch.

  • I've refactored the browserstack workers logic and queue to reuse browser instances. This has reduced the testing time of all 3.x browsers from 35-45min to 5-10min! It uses our built-in report API to set the browser URL when a module completes, rather than trying to use the browserstack REST API to change the URL, which had constant errors for little to no reason. Even if it did work though, it was significantly slower.
  • linting is more accurate now for files in test/runner. I didn't have the order right in eslint.config.js
  • sinon 7.x is the latest version that supports IE9

Sample browserstack runs on my repo:

https://github.com/timmywil/jquery/actions/runs/8118427324
https://github.com/timmywil/jquery/actions/runs/8124843564

Testing time should be around 5-10min for all 3.x browsers!

When reviewing, the first 2 commits are almost identical to the main branch versions. It's the following commits that implement changes for 3.x and new improvements.

Checklist

This is a complete rework of our testing infrastructure. The main goal is to modernize and drop deprecated or undermaintained dependencies (specifically, grunt, karma, and testswarm). We've achieved that by limiting our dependency list to ones that are unlikely to drop support any time soon. The new dependency list includes:

- `qunit` (our trusty unit testing library)
- `selenium-webdriver` (for spinning up local browsers)
- `express` (for starting a test server and adding middleware)
  - express middleware includes uses of `body-parser` and `raw-body`
- `yargs` (for constructing a CLI with pretty help text)
- BrowserStack (for running each of our QUnit modules separately in all of our supported browsers)
  - `browserstack-local` (for opening a local tunnel. This is the same package still currently used in the new Browserstack SDK)
  - We are not using any other BrowserStack library. The newest BrowserStack SDK does not fit our needs (and isn't open source). Existing libraries, such as `node-browserstack` or `browserstack-runner`, either do not quite fit our needs, are under-maintained and out-of-date, or are not robust enough to meet all of our requirements. We instead call the [BrowserStack REST API](https://github.com/browserstack/api) directly.

- automatically retries individual modules in case of test failure(s)
- automatically attempts to re-establish broken tunnels
- automatically refreshes the page in case a test run has stalled
- runs all browsers concurrently and uses as many sessions as are available under the BrowserStack plan. It will wait for available sessions if there are none.
- supports filtering the available list of browsers by browser name, browser version, device, OS, and OS version (see `npm run test:unit -- --list-browsers` for more info). It will retrieve the latest matching browser available if any of those parameters are not specified.
- cleans up after itself (closes the local tunnel, stops the test server, etc.)
- Requires `BROWSERSTACK_USERNAME` and `BROWSERSTACK_ACCESS_KEY` environment variables.

- supports running any local browser as long as the driver is installed, including support for headless mode in Chrome, FF, and Edge
- supports running `basic` tests on the latest [jsdom](https://github.com/jsdom/jsdom#readme), which can be seen in action in this PR (see `test:browserless`)
- Node tests will run as before in PRs and all non-dependabot branches, but now includes tests on real Safari in a GH actions macos image instead of playwright-webkit.
- can run multiple browsers and multiple modules concurrently

Other notes:
- Stale dependencies have been removed and all remaining dependencies have been upgraded with a few exceptions:
  - `sinon`: stopped supporting IE in version 10. But, `sinon` has been updated to 9.x.
  - `husky`: latest does not support Node 10 and runs on `npm install`. Needed for now until git builds are migrated to GitHub Actions.
  - `rollup`: latest does not support Node 10. Needed for now until git builds are migrated to GitHub Actions.
- BrowserStack tests are set to run on each `main` branch commit
- `debug` mode leaves Selenium browsers open whether they pass or fail and leaves browsers with test failures open on BrowserStack. The latter is to avoid leaving open too many sessions.
- This PR includes a workflow to dispatch BrowserStack runs on-demand
- The Node version used for most workflow tests has been upgraded to 20.x
- updated supportjQuery to 3.7.1

Run `npm run test:unit -- --help` for CLI documentation

Close jquerygh-5418
- this will soon be replaced by a new GH actions workflow

Close jquerygh-5422
@timmywil timmywil added the Tests label Mar 2, 2024
@timmywil timmywil added this to the 3.7.2 milestone Mar 2, 2024
@timmywil timmywil requested a review from mgol March 2, 2024 01:16
timmywil added 2 commits March 2, 2024 10:00
- add support for latest and latest-\d+ in test runner
- also fix an issue with retries
- add support for specifying non-final browser versions
- esm -> amd in the test runner for 3.x
Copy link
Member
@mgol mgol left a comment

Choose a reason for hiding this comment

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

Awesome results!

I left some comments.

As for:

It uses our built-in report API to set the browser URL when a module completes, rather than trying to use the browserstack REST API to change the URL, which had constant errors for little to no reason.

Can you explain it in a bit more detail to me?

@timmywil
Copy link
Member Author
timmywil commented Mar 5, 2024

It uses our built-in report API to set the browser URL when a module completes, rather than trying to use the browserstack REST API to change the URL, which had constant errors for little to no reason.

Can you explain it in a bit more detail to me?

Sure, if you look at this line, notice that any response from the report function is returned in the report API response. The only times we return responses are in response to the runEnd request:

listeners.js handles the response by taking the test URL and setting the window location.

Trying to use BrowserStack's REST API to change the URL on the worker was slower and incredibly error-prone.

@timmywil
Copy link
Member Author
timmywil commented Mar 5, 2024

I've incorporate the feedback in #5428 as well.

Copy link
Member
@mgol mgol left a comment

Choose a reason for hiding this comment

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

Just a few comments (please have a look) but nothing blocking so I'm approving as well.

Copy link
Member
@mgol mgol left a comment

Choose a reason for hiding this comment

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

No further comments, looks good!

@timmywil timmywil merged commit ef434cd into jquery:3.x-stable Mar 5, 2024
@timmywil timmywil deleted the migrate-test-3.x branch March 5, 2024 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

2 participants
0