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.

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

Move tests which require chrono-tz feature from arrow-cast to arrow #3222

Merged
merged 7 commits into from
Nov 30, 2022

Conversation

viirya
Copy link
Member

@viirya viirya commented Nov 29, 2022

Which issue does this PR close?

Closes #3219.
Closes #3220.
Closes #3221.

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

@github-actions github-actions bot added the arrow Changes to the arrow crate label Nov 29, 2022
Copy link
Contributor

@tustvold tustvold left a comment

Choose a reason for hiding this comment

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

Could we instead move these to be integration tests in arrow-rs. Otherwise the inconsistent feature flags will result lots of recompilation when switching crates and targets?

@viirya
Copy link
Member Author

viirya commented Nov 29, 2022

Okay.

@viirya viirya changed the title Re-enable some tests in arrow-cast crate Move some tests from arrow-cast crate to arrow Nov 29, 2022
@viirya viirya changed the title Move some tests from arrow-cast crate to arrow Move some tests from arrow-cast to arrow Nov 29, 2022
@@ -3614,7 +3616,6 @@ mod tests {
}

#[test]
#[cfg(not(feature = "force_validate"))]
Copy link
Member Author

@viirya viirya Nov 29, 2022

Choose a reason for hiding this comment

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

For the tests which depend on not "force_validate", I just removed the effectless cfg because that feature doesn't exist in the crate.

@viirya viirya changed the title Move some tests from arrow-cast to arrow Move tests which require chrono-tz feature from arrow-cast to arrow Nov 29, 2022
Copy link
Contributor

@tustvold tustvold left a comment

Choose a reason for hiding this comment

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

Minor nit concerning feature compilation

arrow/tests/array_cast.rs Outdated Show resolved Hide resolved
@tustvold
Copy link
Contributor

Strangely I can't see this in the CI output, but running cargo test -p arrow --features=force_validate,prettyprint,ipc_compression,ffi,dyn_cmp_dict,dyn_arith_dict,chrono-tz locally it runs the test?

@viirya
Copy link
Member Author

viirya commented Nov 30, 2022

I see it in the CI output at https://github.com/apache/arrow-rs/actions/runs/3586065699/jobs/6034811204:

     Running tests/array_cast.rs (target/debug/deps/array_cast-6a755e3bff422147)

running 3 tests
test test_cast_timestamp_to_string ... ok
test test_timestamp_cast_utf8 ... ok
test test_can_cast_types ... ok

test result: ok. 3 passed; 0 failed; 0 ignored; 0 measured; 0 filtered out; finished in 0.02s

@tustvold
Copy link
Contributor

Aah I was using the browser search not the one built into the UI 👍

@tustvold tustvold merged commit 9538c26 into apache:master Nov 30, 2022
@ursabot
Copy link

ursabot commented Nov 30, 2022

Benchmark runs are scheduled for baseline = 6bd559f and contender = 9538c26. 9538c26 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-rs-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arrow Changes to the arrow crate
Projects
None yet
3 participants