tests: use the same browser instance across tests and better use explicit resource management#2901
tests: use the same browser instance across tests and better use explicit resource management#2901marvinhagemeister merged 10 commits intodenoland:mainfrom iuioiua:browser-instance
Conversation
| args: [ | ||
| "--window-size=1280,7201", | ||
| ...((Deno.env.get("CI") && Deno.build.os === "linux") | ||
| ? ["--no-sandbox"] |
There was a problem hiding this comment.
I would recommend removing app armor instead of enabling --no-sandbox
There was a problem hiding this comment.
What is the advantage? Genuine question.
There was a problem hiding this comment.
Or do you think that's the issue here?
There was a problem hiding this comment.
The biggest reason is it cleans up the codebase. I've heard of really obscure bugs not being caught in CI because of --no-sandbox, but I've never seen one myself so take that with a grain of salt.
tests/test_utils.tsx
Outdated
| const browser = await launch({ | ||
| args: ["--window-size=1280,720"], | ||
| headless: true, | ||
| }); | ||
|
|
||
| addEventListener("unload", async () => { | ||
| await browser.close(); | ||
| }); |
There was a problem hiding this comment.
@lino-levan, when I use await using, tests hang.
.github/workflows/ci.yml
Outdated
| - name: Type check project | ||
| run: deno task check:types | ||
|
|
||
| - name: Disable AppArmor |
There was a problem hiding this comment.
I think this is worth trying.
iuioiua
changed the title
iuioiua
changed the title
|
PTAL @marvinhagemeister |
|
@marvinhagemeister, are you able to please take a look? It'd be great to have this bump in test performance. |
3 participants
On my machine, reduces time taken to perform
deno task testfrom ~1m20s to ~30s.Inspired by and supersedes #2110.