Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

chore: Mark flaky web-sveltekit tests as manual#63874

Merged
varungandhi-src merged 2 commits into
mainfrom
vg/comment-tests
Jul 17, 2024
Merged

chore: Mark flaky web-sveltekit tests as manual#63874
varungandhi-src merged 2 commits into
mainfrom
vg/comment-tests

Conversation

@varungandhi-src

Copy link
Copy Markdown
Contributor

Not sure if there's a cleaner way to disable these?

Test plan

n/a

@cla-bot cla-bot Bot added the cla-signed label Jul 17, 2024
@github-actions github-actions Bot added team/graph Graph Team (previously Code Intel/Language Tools/Language Platform) team/product-platform labels Jul 17, 2024
@varungandhi-src varungandhi-src changed the title chore: Comment out flaky web-sveltekit tests chore: Mark flaky web-sveltekit tests as manual Jul 17, 2024
@varungandhi-src varungandhi-src merged commit f3cc352 into main Jul 17, 2024
@varungandhi-src varungandhi-src deleted the vg/comment-tests branch July 17, 2024 10:18
bahrmichael referenced this pull request Jul 17, 2024
The unit tests for web-sveltekit have become flaky, and have therefore
been set to run only manually:
https://github.com/sourcegraph/sourcegraph/pull/63874

By running them via `sg ci bazel` I noticed in the waterfall view, that
the tests are taking slightly above one minute
(https://buildkite.com/sourcegraph/sourcegraph/builds/282732/waterfall),
and that previous tests failed because it exceeded a timeout of 1
minute: https://buildkite.com/sourcegraph/sourcegraph/builds/282684

It looks to me like the CI agents are a bit less powerful, and therefore
take longer than our local test runs (which usually don't exceed 10
seconds).

I have not investigated parallelizing the vitest workers, because for
web-sveltekit we currently only have a vite config, but not a vitest
config. More context on that here:
https://github.com/sourcegraph/sourcegraph/pull/60927

## Test plan

CI

## Changelog

<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
bahrmichael referenced this pull request Jul 18, 2024
The goal of this PR is to increase the stability of web-sveltekit
e2e-tests so that we don't have to rely on manual runs anymore. They
have previously been disabled due to a high number of failures:
https://github.com/sourcegraph/sourcegraph/pull/63874

---

To improve the stability of web-sveltekit e2e-tests, I used `sg ci bazel
test //client/web-sveltekit:e2e_test --runs_per_test=N` with N=5,10,15
to see which tests break under different levels of pressure on the
machine. The logs looked like it was mostly timeouts, that got worse
when increasing N. That means we can check where tests will break due to
timeouts, but we don't really need to raise timeouts so far that it
would work with N=20.

With N=5, 10 we get a good understanding if our timeouts are high
enough.

You can see two CI runs here after applying higher timeouts and skipping
a consistently failing test:

- N=5:
https://buildkite.com/sourcegraph/sourcegraph/builds/283011/waterfall
- N=10:
https://buildkite.com/sourcegraph/sourcegraph/builds/283013/waterfall

---

From logs of some other run that I don't have the link to anymore, we
can see that some tests take up to 50s so a timeout of 60s (instead of
the default 30s) for CI should be a good new ceiling.

```
Slow test file: [chromium] › src/routes/[...repo=reporev]/(validrev)/(code)/-/blob/[...path]/page.spec.ts (48.9s)
--
  | Slow test file: [chromium] › src/routes/[...repo=reporev]/(validrev)/(code)/page.spec.ts (45.0s)
  | Slow test file: [chromium] › src/routes/search/page.spec.ts (40.6s)
  | Slow test file: [chromium] › src/routes/[...repo=reporev]/(validrev)/(code)/-/tree/[...path]/page.spec.ts (31.7s)
  | Slow test file: [chromium] › src/routes/layout.spec.ts (31.5s)

```

## Test plan

CI

## Changelog

<!-- OPTIONAL; info at
https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c
-->
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed team/graph Graph Team (previously Code Intel/Language Tools/Language Platform) team/product-platform

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants