-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Conversation
Please use draft PRs to avoid sending notifications 🙇 |
048c35c
to
2df9ea8
Compare
2df9ea8
to
5692679
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, the Phabricator e2e tests could also use Driver
. https://github.com/sourcegraph/sourcegraph/pull/4226
cf98054
to
31f5851
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still a lot of non-e2e-prefixed selectors but I assume those existed before.
} | ||
}) | ||
}, | ||
5 * 1000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we pass the same timeout to every test if we also set the timeout at the top?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That timeout applies generally, but the search test behavior is explicitly to timeout after 5s (as taking longer than 5s to serve the query indicates a broken state).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would have been good to mention in a comment - this timeout seems to have been copy-pasted for e.g. external service tests too
This adds a regression test file
regression.test.tsx
that defines regression tests. This is separate from theindex.e2e.test.tsx
file, which defines e2e tests that run in CI. The reason that the two are separate is that we expect there to be a lot of regression tests and it would be prohibitively expensive to run them each time in CI and they might be flakier than our tolerance for the in-CI e2e tests.Run it with
GITHUB_TOKEN=$TOKEN yarn run test-regression
in theweb
directory.regression.test.tsx
currently includes test for one of the search use cases, but will eventually grow to cover most of the Release test grid.cc @lguychard because you made a suggestion to use Cucumber.io. This PR does not use Cucumber. It may be the right solution for us down the road, but first I wanted to implement the tests directly in JS to see if this adequately addresses our needs.