update local testing workflow to allow for parallelism, improved dx #3707
Conversation
| @@ -15,7 +15,7 @@ | |||
| "dev-web": "npm run dev -w web-local", | |||
| "dev-runtime": "go run cli/main.go start dev-project --no-ui", | |||
| "clean": "rm -rf dev-project", | |||
| "test": "npm run test -w web-common && npm run test -w web-local && npm run test -w web-auth" | |||
| "test": "npm run test -w web-common & npm run test -w web-auth & make cli && npm run test -w web-local" | |||
There was a problem hiding this comment.
qq: typo at single "&" at web-common & npm run test?
There was a problem hiding this comment.
The intention is to run npm run test -w web-common, npm run test -w web-auth and make cli concurrently (separated by a single &) since they aren't dependent on each other. After that completes, npm run test -w web-local will run.
The two unit tests are pretty fast, but I just want to start make cli as soon as possible since it takes the longest.
There was a problem hiding this comment.
gotcha, makes sense, great idea!
|
I think this all makes sense and looks like some nice workflow upgrades. The only question I would have is whether overriding the test object will limit our flexibility in the future if there are any cases in which want to do playwright testing without the runtime, but that is highly speculative, and I don't think that should block us from merging this nice set of improvements. Consider me as a positive review for this PR, but I have also tagged @AdityaHegde for review since he's more familiar with a bunch of this lower level testing+backend stuff than I am. Thanks @briangregoryholmes! |
|
Totally valid. To limit disruption, I kept the same I agree that there's some benefit to testing the application code independent of the runtime, where you spin up just a single backend and then have workers run different instances of the frontend. That is a larger lift, but something I think would be valuable in development. Hypothetically you wouldn't have to run |
| @@ -15,11 +15,11 @@ async function assertAAboveB(locA: Locator, locB: Locator) { | |||
| } | |||
|
|
|||
| test.describe("leaderboard and dimension table sorting", () => { | |||
| startRuntimeForEachTest(); | |||
| // startRuntimeForEachTest(); | |||
There was a problem hiding this comment.
Nit: delete this leftover comment
web-local/test/ui/utils/test.ts
Outdated
|
|
||
| export const test = base.extend({ | ||
| page: async ({ page }, use) => { | ||
| const TEST_PORT = await getOpenPort(); |
There was a problem hiding this comment.
Curious how this will work if the port is released immediately.
|
@briangregoryholmes : @ericpgreen2 : This seems like a helpful PR. If there is not much pending work here, Lets fix conflicts and merge. |
…3707) * update local testing workflow to allow for parallelism, improved dx * update comment * remove extraneous comment * move file/directory removal to before process exit * prettier
The purpose of this PR is to improve, in a small way, Rill's local testing workflow during development.
When developing at the application level, the current approach to testing necessitates running
make clibefore every test run. As such, this PR updates thenpm run testscript to include that command. Additionally, it runs theweb-authandweb-commonunit tests concurrently with this build process, running theweb-commone2e tests after this process completes.I believe there is additional room for improvement in this regard (the runtime certainly doesn't need to be re-compiled every time), but this PR is intended to be limited in its disruption.
This PR also creates a new test fixture by extending the built in Playwright test object rather than using the beforeEach/afterEach pattern. This allows us to encapsulate/extend the testing environment without having to import the
startRuntimeForEachTestfunction. This new approach also allows for a very straightforward way of dynamically assigning ports so that tests can be run in parallel. It also sets us up to use the Page Objet Model https://playwright.dev/docs/pom more easily in the future, allowing us to capture all the utils and common operations under a single class.Finally, the Playwright config file has been updated to allow for tests to execute in parallel when running locally. It does not change any behavior when testing in CI.