Skip to content

Browser tests stability #4855

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 4 commits into from
Jun 16, 2025
Merged

Browser tests stability #4855

merged 4 commits into from
Jun 16, 2025

Conversation

mstoykov
Copy link
Contributor

What?

Try to make browser tests more stable once again.

Part of the focus here was to run tests one by one and try to figure out which tests are leaving browsers that never die.

As part of this some other small fixes were added.

The startIteration was removed mostly due to it mostly being used alongside newTestBrowser, leading to multiple browsers started per test.

Why?

Browser test are often the reason for CI tests to fail, and most of them seem to be due to performance problems - likely due to too many browsers running at the same time.

Checklist

  • I have performed a self-review of my code.
  • I have commented on my code, particularly in hard-to-understand areas.
  • I have added tests for my changes.
  • I have run linter and tests locally (make check) and all pass.

Checklist: Documentation (only for k6 maintainers and if relevant)

Please do not merge this PR until the following items are filled out.

  • I have added the correct milestone and labels to the PR.
  • I have updated the release notes: link
  • I have updated or added an issue to the k6-documentation: grafana/k6-docs#NUMBER if applicable
  • I have updated or added an issue to the TypeScript definitions: grafana/k6-DefinitelyTyped#NUMBER if applicable

Related PR(s)/Issue(s)

mstoykov added 4 commits June 16, 2025 13:41
This test doesn't really have any reason to be parallel, but it likely
was very flaky due to it.
This seems to have some cases where it will not close the browser
otherwise. It doesn't seem to have any other side effects.
This seems to be completely unused - maybe it was needed before.
This both makes it so we do not have two different ways of starting
browser in the test.

But also doesn't start separate browsers just to setup file servers and
then others through startIteration.
@mstoykov mstoykov added this to the v1.1.0 milestone Jun 16, 2025
@mstoykov mstoykov requested a review from a team as a code owner June 16, 2025 10:47
@mstoykov mstoykov requested review from ankur22 and AgnesToulet and removed request for a team June 16, 2025 10:47
@mstoykov
Copy link
Contributor Author

Please review commit by commit as otherwise it will be a lot harder.

I can break this up in separate PRs if that will be better - I expected a lot more of the first PRs, which meant I didn't want to break it down.

@mstoykov mstoykov merged commit df23c37 into master Jun 16, 2025
39 checks passed
@mstoykov mstoykov deleted the browserTestsStability branch June 16, 2025 12:40
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