Skip to content

tests: use the same browser instance across tests and better use explicit resource management#2901

Merged
marvinhagemeister merged 10 commits into
freshframework:mainfrom
iuioiua:browser-instance
May 21, 2025
Merged

tests: use the same browser instance across tests and better use explicit resource management#2901
marvinhagemeister merged 10 commits into
freshframework:mainfrom
iuioiua:browser-instance

Conversation

@iuioiua

@iuioiua iuioiua commented May 7, 2025

Copy link
Copy Markdown
Contributor

On my machine, reduces time taken to perform deno task test from ~1m20s to ~30s.

Inspired by and supersedes #2110.

Comment thread tests/test_utils.tsx Outdated
Comment on lines +39 to +46
const browser = await launch({
args: ["--window-size=1280,720"],
headless: true,
});

addEventListener("unload", async () => {
await browser.close();
});

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@lino-levan, when I use await using, tests hang.

Comment thread .github/workflows/ci.yml Outdated
- name: Type check project
run: deno task check:types

- name: Disable AppArmor

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this is worth trying.

@iuioiua iuioiua changed the title test: use the same browser instance across tests test: use the same browser instance across tests and better use explicit resource management May 8, 2025
@iuioiua iuioiua marked this pull request as ready for review May 8, 2025 00:36
@iuioiua iuioiua changed the title test: use the same browser instance across tests and better use explicit resource management tests: use the same browser instance across tests and better use explicit resource management May 12, 2025
@iuioiua

iuioiua commented May 12, 2025

Copy link
Copy Markdown
Contributor Author

PTAL @marvinhagemeister

Comment thread tests/test_utils.tsx Outdated
@iuioiua iuioiua requested a review from marvinhagemeister May 12, 2025 23:08
@iuioiua

iuioiua commented May 18, 2025

Copy link
Copy Markdown
Contributor Author

@marvinhagemeister, are you able to please take a look? It'd be great to have this bump in test performance.

@marvinhagemeister marvinhagemeister left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@marvinhagemeister marvinhagemeister merged commit da8879d into freshframework:main May 21, 2025
7 checks passed
@iuioiua iuioiua deleted the browser-instance branch May 21, 2025 09:42
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.

2 participants