Skip to content

Commit

Permalink
don't use std abs_diff, put it in test_utils instead, run tests with …
Browse files Browse the repository at this point in the history
…msrv in action (#596)

<!--
Thanks for sending a pull request!  Here are some tips for you:
1. If this is your first time, please read our contributor guidelines:
https://github.com/delta-incubator/delta-kernel-rs/blob/main/CONTRIBUTING.md
2. Run `cargo t --all-features --all-targets` to get started testing,
and run `cargo fmt`.
  3. Ensure you have added or run the appropriate tests for your PR.
4. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP]
Your PR title ...'.
  5. Be sure to keep the PR description updated to reflect all changes.
-->

## What changes are proposed in this pull request?
<!--
Please clarify what changes you are proposing and why the changes are
needed.
The purpose of this section is to outline the changes, why they are
needed, and how this PR fixes the issue.
If the reason for the change is already explained clearly in an issue,
then it does not need to be restated here.
1. If you propose a new API or feature, clarify the use case for a new
API or feature.
  2. If you fix a bug, you can clarify why it is a bug.
-->

- create an abs_diff function in test_utils (same code as in std)
- use that in the two places we wanted it instead
- also add a workflow to make sure that we can always compile against
msrv


<!--
Uncomment this section if there are any changes affecting public APIs:
### This PR affects the following public APIs

If there are breaking changes, please ensure the `breaking-changes`
label gets added by CI, and describe why the changes are needed.

Note that _new_ public APIs are not considered breaking.
-->


## How was this change tested?
<!--
Please make sure to add test cases that check the changes thoroughly
including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please
clarify how you tested, ideally via a reproducible test documented in
the PR description.
-->

- Existing tests pass
- New workflow passes
  • Loading branch information
nicklan authored Dec 13, 2024
1 parent 702e12f commit 71deaa3
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 3 deletions.
28 changes: 28 additions & 0 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,34 @@ jobs:
cargo msrv --path derive-macros/ verify --all-features
cargo msrv --path ffi/ verify --all-features
cargo msrv --path ffi-proc-macros/ verify --all-features
msrv-run-tests:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Install minimal stable and cargo msrv
uses: actions-rs/toolchain@v1
with:
profile: default
toolchain: stable
override: true
- uses: Swatinem/rust-cache@v2
- name: Install cargo-msrv
shell: bash
run: |
cargo install cargo-msrv --locked
- name: Get rust-version from Cargo.toml
id: rust-version
run: echo "RUST_VERSION=$(cargo msrv show --path kernel/ --output-format minimal)" >> $GITHUB_ENV
- name: Install specified rust version
uses: actions-rs/toolchain@v1
with:
toolchain: ${{ env.RUST_VERSION }}
profile: minimal
- name: run tests
run: |
pushd kernel
echo "Testing with $(cargo msrv show --output-format minimal)"
cargo +$(cargo msrv show --output-format minimal) test
docs:
runs-on: ubuntu-latest
env:
Expand Down
4 changes: 2 additions & 2 deletions kernel/src/engine/default/filesystem.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ mod tests {
use object_store::memory::InMemory;
use object_store::{local::LocalFileSystem, ObjectStore};

use test_utils::delta_path_for_version;
use test_utils::{abs_diff, delta_path_for_version};

use crate::engine::default::executor::tokio::TokioBackgroundExecutor;
use crate::engine::default::DefaultEngine;
Expand Down Expand Up @@ -241,7 +241,7 @@ mod tests {
assert!(!files.is_empty());
for meta in files.into_iter() {
let meta_time = Duration::from_millis(meta.last_modified.try_into().unwrap());
assert!(meta_time.abs_diff(begin_time) < Duration::from_secs(10));
assert!(abs_diff(meta_time, begin_time) < Duration::from_secs(10));
}
}
#[tokio::test]
Expand Down
4 changes: 3 additions & 1 deletion kernel/src/engine/sync/fs_client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ mod tests {
use itertools::Itertools;
use url::Url;

use test_utils::abs_diff;

use super::SyncFilesystemClient;
use crate::FileSystemClient;

Expand Down Expand Up @@ -111,7 +113,7 @@ mod tests {
assert!(!files.is_empty());
for meta in files.iter() {
let meta_time = Duration::from_millis(meta.last_modified.try_into()?);
assert!(meta_time.abs_diff(begin_time) < Duration::from_secs(10));
assert!(abs_diff(meta_time, begin_time) < Duration::from_secs(10));
}
Ok(())
}
Expand Down
10 changes: 10 additions & 0 deletions test-utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,3 +106,13 @@ pub fn into_record_batch(engine_data: Box<dyn EngineData>) -> RecordBatch {
.unwrap()
.into()
}

/// We implement abs_diff here so we don't have to bump our msrv.
/// TODO: Remove and use std version when msrv >= 1.81.0
pub fn abs_diff(self_dur: std::time::Duration, other: std::time::Duration) -> std::time::Duration {
if let Some(res) = self_dur.checked_sub(other) {
res
} else {
other.checked_sub(self_dur).unwrap()
}
}

0 comments on commit 71deaa3

Please sign in to comment.