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

wip: speed up builds w/ parallelism #3444

Closed
wants to merge 5 commits into from

Conversation

toolmantim
Copy link
Contributor

@toolmantim toolmantim commented Mar 17, 2024

Build time is now 13 min. Last time (pre-windows builds) we got it back down to 4-5 min by bumping macOS instance size. We've hit the limits for bumping instance on macOS, so we'll need to split tasks (and probably bump instances for Windows & Ubuntu, which require GitHub org admin access).

But first let's see if we can parallelize on-machine, though I'm not hopeful: in general I've found this to be a fool's errand — parallelising e2e tests is a notorious change for introducing flakiness.

Test plan

n/a

@toolmantim toolmantim changed the title speed up builds wip: speed up builds w/ parallelism Mar 17, 2024
@toolmantim toolmantim force-pushed the tl/support-vitest-parallelism branch from fedc4f1 to 382b0a4 Compare March 18, 2024 00:31
toolmantim and others added 3 commits March 17, 2024 21:11
This might be because the global `loggedEvents` is written to by several parallel tests at once. I'm not sure. I would have thought that each test case would be run in a separate process, but that does not necessarily seem to be the case.
@sqs sqs mentioned this pull request Mar 18, 2024
@sqs
Copy link
Member

sqs commented Mar 18, 2024

@toolmantim I made a bit more progress here. See #3449 in case it's helpful.

@toolmantim toolmantim force-pushed the tl/support-vitest-parallelism branch from ba040a3 to 05cc633 Compare March 18, 2024 10:46
@toolmantim toolmantim force-pushed the tl/support-vitest-parallelism branch from 762c854 to dddc414 Compare March 18, 2024 11:05
@sqs
Copy link
Member

sqs commented Mar 19, 2024

@toolmantim ooh nice fix for telemetry. Is this working for worker > 1 for you?

@toolmantim
Copy link
Contributor Author

Yeah, but not reliably without retries:

$ pnpm run test:e2e -j 2
...
  Slow test file: chat-atFile.test.ts (24.5s)
  Slow test file: command-custom.test.ts (20.5s)
  Slow test file: local-embeddings.test.ts (15.9s)
  Consider splitting slow test files to speed up parallel execution
  4 skipped
  37 passed (1.4m)

$ pnpm run test:e2e -j 2
  Slow test file: command-core.test.ts (33.6s)
  Slow test file: chat-atFile.test.ts (23.5s)
  Slow test file: command-custom.test.ts (19.3s)
  Slow test file: local-embeddings.test.ts (16.2s)
  Consider splitting slow test files to speed up parallel execution
  1 flaky
    command-core.test.ts:100:3 › Generate Unit Test Command (Edit) ─────────────────────────────────
  4 skipped
  36 passed (1.6m)

$ pnpm run test:e2e -j 3
...
  Slow test file: command-custom.test.ts (32.7s)
  Slow test file: chat-atFile.test.ts (27.9s)
  Slow test file: local-embeddings.test.ts (19.8s)
  Consider splitting slow test files to speed up parallel execution
  1 flaky
    command-custom.test.ts:145:3 › execute custom commands with context defined in cody.json ───────
  4 skipped
  36 passed (1.1m)

$ pnpm run test:e2e -j 4
...
  Slow test file: command-custom.test.ts (1.0m)
  Slow test file: chat-atFile.test.ts (32.4s)
  Slow test file: code-actions.test.ts (31.9s)
  Slow test file: local-embeddings.test.ts (18.6s)
  Slow test file: command-core.test.ts (15.2s)
  Consider splitting slow test files to speed up parallel execution
  3 flaky
    code-actions.test.ts:16:3 › code action: explain ───────────────────────────────────────────────
    command-custom.test.ts:36:3 › create a new user command via the custom commands menu ───────────
    command-custom.test.ts:247:3 › open and delete cody.json from the custom command menu ──────────
  4 skipped
  34 passed (1.6m)

I doubt we'll get this stable… I think it's best we just shard and bump instances sizes instead. It's also very easy to scale shards.

@toolmantim
Copy link
Contributor Author

Actually, after some more battle testing I am getting local flakiness with 1 worker too. I'll cherry-pick f85a049 and run things again.

@toolmantim toolmantim closed this Mar 19, 2024
@toolmantim toolmantim deleted the tl/support-vitest-parallelism branch March 19, 2024 23:05
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