Skip to content

Split up the big main test into many smaller ones #1304

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.

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

Open
wants to merge 7 commits into
base: main
Choose a base branch
from

Conversation

adamchalmers
Copy link
Contributor

@adamchalmers adamchalmers commented Jul 22, 2025

PR currently has a problem with logging in.

Many of the tests require a logged-in Zoo CLI. That's fine in the current model, where all tests run in one process sequentially. But with nextest, each test runs in its own process. So each test has to log in, then run its test. That was fine but somehow, each test would successfully run its login code, and then try to run the relevant test (e.g. zoo kcl mass). But that relevant test would fail, saying you weren't logged in. Seems like the test wasn't passing the logged-in status into the next CLI invocation. Idk what to do about it.

@adamchalmers adamchalmers requested a review from jessfraz as a code owner July 22, 2025 18:17
@adamchalmers adamchalmers force-pushed the achalmers/split-up-tests2 branch from 1157a19 to 90fe3d0 Compare July 22, 2025 18:29
@adamchalmers adamchalmers force-pushed the achalmers/split-up-tests2 branch from 2ce47c5 to 528f4d4 Compare July 22, 2025 20:34
name: "version".to_string(),
args: vec!["zoo".to_string(), "version".to_string()],
want_out: format!(
"zoo {} )({})\n{}",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a syntax error in the format string. The parentheses are incorrectly ordered - it currently reads zoo {} )({})\n{} but should be zoo {} ({})\n{}. This will cause a runtime error when formatting the version string.

Suggested change
"zoo {} )({})\n{}",
"zoo {} ({})\n{}",

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

@adamchalmers adamchalmers force-pushed the achalmers/split-up-tests2 branch from cf2c70b to 76a3f05 Compare July 23, 2025 21:23
@adamchalmers adamchalmers mentioned this pull request Jul 23, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant