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.

Sign up for GitHub

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

Jump to bottom

Rust tests should use 1 engine instance #2580

Open
adamchalmers opened this issue Jun 3, 2024 · 2 comments · May be fixed by #2627
Open

Rust tests should use 1 engine instance #2580

adamchalmers opened this issue Jun 3, 2024 · 2 comments · May be fixed by #2627
Assignees
@adamchalmers
Labels
dev Issues related to development of the app.

Comments

@adamchalmers
Copy link
Collaborator

adamchalmers commented Jun 3, 2024 •
edited
Loading

Background

Currently every Rust unit test creates a new kittycad client, starting a new session with the engine.

Problem

This seems inefficient -- it would add a lot of latency from starting and stopping engine sessions.

Solution

Before running any tests, start a test server. The test server has one HTTP endpoint which takes a KCL program, executes it, and responds with a PNG snapshot. It does this by running a KCL executor.

Each test, instead of making its own KCL executor, just sends programs to this HTTP server. This way our suite of 100 tests only connects to the engine once, instead of 100 times -- should be much faster.
@jessfraz jessfraz added the dev Issues related to development of the app. label Jun 3, 2024
@adamchalmers adamchalmers mentioned this issue Jun 6, 2024
Merged
@adamchalmers adamchalmers self-assigned this Jun 6, 2024
@adamchalmers
Copy link
Collaborator Author

adamchalmers commented Jun 6, 2024 •
edited
Loading

Current plan is:

  • The CI image (and local dev, if you're running tests locally) will require starting a server. Start it with cargo run kcl-test-server.
  • That server connects to the engine, and listens for incoming KCL programs over localhost:3333. When it gets a HTTP request, it gets a KCL file from the request body, executes it on the engine, and serves a PNG snapshot in response.
  • The unit tests will send their KCL files over localhost:3333 and get a PNG file back. Then call 2020 on that PNG as usual.

Nextest has experimental support for setup scripts (docs, tracking issue: nextest-rs/nextest#978). If we use it, then Nextest would automatically start the test server if you run a test that requires it. Could be nice.

If we don't want to use the experimental nextest feature, then we just kcl-test-server in GitHub Actions, and print a nice error message in the unit tests if the server isn't found.

I've got a POC here: #2627

@adamchalmers adamchalmers linked a pull request Jun 6, 2024 that will close this issue
Draft
@adamchalmers
Copy link
Collaborator Author

Good news, on my POC, tests run in only 56% of the time they used to take (2.4s vs 1.4s). This adds up to quite a lot of savings when spread over 100 tests!

@adamchalmers adamchalmers mentioned this issue Jun 17, 2024
Merged
adamchalmers added a commit that referenced this issue Jun 18, 2024
@adamchalmers
KCL execution server (#2686)
c612730 
Adds a new library, the kcl-test-server. It lets you easily start a HTTP server with one endpoint, which accepts JSON. The JSON body contains a KCL program and a test name. The server has a pool of active engine sessions, and when it gets a KCL program, it executes it on one of those engine sessions.

This addresses part of #2580 but currently the sketch-on-face tests don't pass with this new test server yet.

This is a library, not a binary, because I want to use it in both the wasm-lib unit tests and in the zoo CLI.
@jessfraz jessfraz modified the milestone: v1 Modeling App Launch Jun 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@adamchalmers adamchalmers

Labels
dev Issues related to development of the app.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Reuse engine session between unit tests KittyCAD/modeling-app
2 participants
@jessfraz @adamchalmers