Skip to content

Move to ZarrTestBuilder#243

Open
ppinchuk wants to merge 15 commits intomainfrom
pp/test_cleanup
Open

Move to ZarrTestBuilder#243
ppinchuk wants to merge 15 commits intomainfrom
pp/test_cleanup

Conversation

@ppinchuk
Copy link
Copy Markdown
Collaborator

@ppinchuk ppinchuk commented Apr 9, 2026

Moved all the tests I could find that were creating a Zarr file for testing to use ZarrTestBuilder. Deprecated the old functions and approach in favor of this cleaner setup. Thanks @castelao!!

@ppinchuk ppinchuk added this to the Long-term plans milestone Apr 9, 2026
@ppinchuk ppinchuk self-assigned this Apr 9, 2026
@ppinchuk ppinchuk added topic-rust-general Issues/pull requests related to rust chore Maintenance work that does not impact the end user p-low Priority: low rust Pull requests that update rust code labels Apr 9, 2026
Copilot AI review requested due to automatic review settings April 9, 2026 06:00
@ppinchuk ppinchuk requested a review from castelao as a code owner April 9, 2026 06:00
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors Rust-side test dataset creation to consistently use dataset::samples::ZarrTestBuilder, replacing older ad-hoc Zarr construction helpers and updating dependent tests to the new APIs (including an explicit band dimension).

Changes:

  • Updated ZarrTestBuilder to model 3D arrays (band, y, x) and expanded helper constructors accordingly.
  • Migrated multiple tests to use ZarrTestBuilder / new samples::*_zarr(...) helpers instead of the removed legacy builders.
  • Adjusted dataset and lazy-subset tests to use 3D ArraySubset ranges and validate 3D shapes.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
crates/revrt/src/routing/features/samples.rs Updates sample docs to reference the new random multi-variable helper.
crates/revrt/src/lib.rs Migrates routing tests to the new samples helpers and builder-based dataset creation.
crates/revrt/src/dataset/samples.rs Extends ZarrTestBuilder to include band dimension + new helpers; removes legacy builders.
crates/revrt/src/dataset/mod.rs Updates dataset tests to use builder-based Zarr fixtures and new helper signatures.
crates/revrt/src/dataset/lazy_subset.rs Updates async lazy-subset tests for 3D (band, y, x) subsets and shapes.
crates/revrt/src/cost.rs Updates cost tests to build Zarr fixtures via ZarrTestBuilder.

@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Apr 9, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 99.34%. Comparing base (4dfff67) to head (8b82867).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #243   +/-   ##
=======================================
  Coverage   99.34%   99.34%           
=======================================
  Files          25       25           
  Lines        2765     2765           
  Branches      320      320           
=======================================
  Hits         2747     2747           
  Misses          9        9           
  Partials        9        9           
Flag Coverage Δ
unittests 99.34% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

castelao
castelao previously approved these changes Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

chore Maintenance work that does not impact the end user p-low Priority: low rust Pull requests that update rust code topic-rust-general Issues/pull requests related to rust

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants