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

seed database used for test suite maybe should live in ./target? #548

Closed
davepacheco opened this issue Dec 23, 2021 · 1 comment · Fixed by #563
Closed

seed database used for test suite maybe should live in ./target? #548

davepacheco opened this issue Dec 23, 2021 · 1 comment · Fixed by #563
Assignees
@smklein
Labels
Testing & Analysis Tests & Analyzers

Comments

@davepacheco
Copy link
Collaborator

While debugging #540, I realized that I was running cargo test from two different clones of omicron with the same value of $TMPDIR in my environment, and that they could totally stomp on each other if one of their "build" steps initialized $TMPDIR/crdb-base while the other one's test suite was running trying to create a new database directory from that directory.

If we move the seed directory to ./target, it will at least be scoped to one local clone of Omicron.
@davepacheco
Copy link
Collaborator Author

Another reason: it's surprising that cargo clean doesn't remove the seed directory, since that's generated by the build.

This is obviated for now by #549 but eventually we'll probably want to re-add that and we'll want to address this.

@davepacheco davepacheco mentioned this issue Dec 23, 2021
Open
@smklein smklein self-assigned this Dec 27, 2021
@smklein smklein mentioned this issue Dec 27, 2021
Merged
@smklein smklein added the Testing & Analysis Tests & Analyzers label Dec 27, 2021
smklein added a commit that referenced this issue Jan 3, 2022
@smklein
Generate "crdb-base" directory in target, not temporary directory (#563)
747bcc8 
This new behavior abides by the rules for generating output from a build script, by using the OUT_DIR path available at compile-time.

To ensure that the OUT_DIR path is available to the build script and invocations of test_setup_database, the crates are organized as follows:

- test-utils, the top-level crate, contains functionality for both populating + copying from a seed directory. This is made available to both the build script (in nexus/test-utils) and any tests (in nexus) that want to call test_setup_database. Calling the test-utils version of test_setup_database now explicitly requires providing a path to the seed directory.
- The build script within nexus/test-utils defines an OUT_DIR, and creates the crdb-base directory there. It additionally provides another implementation of test_setup_database, which also embeds the path to the crdb-base directory.
- Most tests use the version of test_setup_database from nexus/test-utils, which embeds the path to crdb-base created from the build script.

Fixes #548
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@smklein smklein

Labels
Testing & Analysis Tests & Analyzers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Generate "crdb-base" directory in target, not temporary directory oxidecomputer/omicron
2 participants
@davepacheco @smklein