Uh oh!
There was an error while loading. Please reload this page.
feat: Add parallelism support for sqlness - #71
Conversation
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
There was a problem hiding this comment.
Copilot reviewed 6 out of 10 changed files in this pull request and generated 2 comments.
Files not reviewed (4)
- sqlness/src/interceptor.rs: Evaluated as low risk
- sqlness/examples/basic.rs: Evaluated as low risk
- sqlness/Cargo.toml: Evaluated as low risk
- sqlness/src/environment.rs: Evaluated as low risk
Comments suppressed due to low confidence (2)
sqlness/src/runner.rs:170
- Ensure that the new parallelism feature is explicitly tested to verify its behavior.
futures::future::join_all(futures).await;
sqlness/examples/bad.rs:19
- The removal of the return statement may affect the intended behavior. Consider adding the return statement back.
Box::new("Unexpected")
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Ruihang Xia <waynestxia@gmail.com>
| /// The id is used to distinguish different database instances in the same environment. | ||
| /// For example, you may want to run the sqlness test in parallel against different instances | ||
| /// of the same environment to accelerate the test. | ||
| async fn start(&self, env: &str, id: usize, config: Option<&Path>) -> Self::DB; |
There was a problem hiding this comment.
Strings can be used for identifiers (id) because they offer flexibility and can accommodate any character-based data, making them suitable for dynamic or user-generated IDs that might include numbers, letters, or special characters.
| let db = self.env_controller.start(&env, config_path).await; | ||
| let run_result = self.run_env(&env, &db).await; | ||
| self.env_controller.stop(&env, db).await; | ||
| let parallelism = self.config.parallelism.max(1); |
There was a problem hiding this comment.
min will become either 0 or 1. This is "take the min/max among two" (really misleading, I've fallen into this many times...)
| let config = ConfigBuilder::default() | ||
| .case_dir(args.case_dir) | ||
| .parallelism(1) |
Rationale
The test time can be very long with increasing test cases. Supporting run them in parallel is a way to relieve this.
Detailed Changes
Support to build multiple environments, and schedule test cases to those env on file level
Test Plan
This change itself is relatively simple, and a major part is for lifetime