cht42 opened a new pull request, #9165:
URL: https://github.com/apache/arrow-rs/pull/9165
# Which issue does this PR close?
- Closes #N/A (internal refactoring - no issue)
# Rationale for this change
Noticed while writing tests in #9144, that the current tests could be
re-written to be easier to read/re-use.
⚠️ FIY, I used claude to refactor those tests, I read the changes and we are
keeping the same test cases.
The Date64 boundary tests in `arrow-arith/src/numeric.rs` had significant
code duplication. Each test function for Date64 operations
(`to_naive_date_opt`, `add_year_months_opt`, `subtract_year_months_opt`,
`add_day_time_opt`, `subtract_day_time_opt`, `add_month_day_nano_opt`,
`subtract_month_day_nano_opt`) repeated similar setup code and boundary checks,
making the test suite harder to maintain and extend.
# What changes are included in this PR?
This PR refactors the Date64 boundary tests by:
1. **Introducing shared constants** for commonly used values:
- `MAX_VALID_DATE`, `MIN_VALID_DATE`, `EPOCH` - NaiveDate constants for
chrono's valid date range
2. **Adding utility functions** to reduce repetition:
- `date_to_millis(year, month, day)` - converts a date to milliseconds
from epoch
- `max_valid_millis()`, `min_valid_millis()`, `year_2000_millis()` -
common millisecond values
3. **Consolidating similar test patterns** into parameterized helper
functions:
- `test_year_month_op()` - tests `add_year_months_opt` and
`subtract_year_months_opt`
- `test_day_time_op()` - tests `add_day_time_opt` and
`subtract_day_time_opt`
- `test_month_day_nano_op()` - tests `add_month_day_nano_opt` and
`subtract_month_day_nano_opt`
4. **Reducing 8 separate test functions to 4** while maintaining the same
test coverage
Net result: **-297 lines** (163 added, 460 removed) with equivalent
functionality.
# Are these changes tested?
Yes - this is a refactoring of existing tests. The same boundary conditions
and edge cases are still tested, just organized more efficiently. Running
`cargo test` confirms all tests pass.
# Are there any user-facing changes?
No. This is an internal test refactoring with no changes to public APIs or
functionality.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
