delta-io / delta-kernel-rs

A native Delta implementation for integration with any query engine
Apache License 2.0
144 stars 41 forks source link

Move some common test-utils to their own crate #477

Closed nicklan closed 3 days ago

nicklan commented 3 days ago

What changes are proposed in this pull request?

Move some useful common utils for building an in-memory delta table into their own crate.

I want to start adding tests for actually reading data to the ffi crate. To do so we need to use an InMemory object store, because all i/o in tokio fails in miri due to it using epoll, which miri doesn't support. So we'll need to build in-memory tables and use the memory:/// url to get a memory backed ObjectStore.

We already do this in read.rs in kernel, but there's no way to get at that code inside the ffi crate. So this moves a bunch of utilities for this sort of thing into its own "test-utils" crate and uses it in read.rs.

I've also cleaned up and renamed things a bit. Most of the code remains unchanged.

Follow-up PR(s) will use this in the ffi crate to test more.

How was this change tested?

Existing tests pass

codecov[bot] commented 3 days ago

Codecov Report

Attention: Patch coverage is 98.61111% with 1 line in your changes missing coverage. Please review.

Project coverage is 79.64%. Comparing base (67aa7d5) to head (b20642a). Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
test-utils/src/lib.rs 98.21% 0 Missing and 1 partial :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #477 +/- ## ========================================== + Coverage 79.56% 79.64% +0.07% ========================================== Files 56 57 +1 Lines 12271 12324 +53 Branches 12271 12324 +53 ========================================== + Hits 9763 9815 +52 Misses 1981 1981 - Partials 527 528 +1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

zachschuermann commented 3 days ago

weird semver fails due to

failed to parse /home/runner/work/delta-kernel-rs/delta-kernel-rs/target/semver-checks/git-9861282e77ba1e3ef3410780d490feab52e90b18/83da59f23e419fe81cc19b13b6750f4c634b28bb/Cargo.toml: no `package` table,
nicklan commented 3 days ago

weird semver fails due to

failed to parse /home/runner/work/delta-kernel-rs/delta-kernel-rs/target/semver-checks/git-9861282e77ba1e3ef3410780d490feab52e90b18/83da59f23e419fe81cc19b13b6750f4c634b28bb/Cargo.toml: no `package` table,

Ahh, it's because it's trying to compare against main which doesn't have test_utils. Will only be an issue for this PR

nicklan commented 3 days ago

nit: is test-utils or test_utils more idiomatic I don't remember? Also does naming not matter since this won't ever make it to crates.io?

Yeah, doesn't really matter, but we've gone camel_case for all the other crates we have, so I've updated it to make it consistent.