[repo-improver] test-coverage: cover check_input_data validation error branches - #263
Open
Mike Tokic (mitokic) wants to merge 2 commits into
Open
Conversation
Add focused testthat coverage for previously untested check_input_data validation branches: missing combo/target/external-regressor columns, non-numeric target, missing and non-date-formatted Date column, uneven day-of-month spacing for month data (and week bypass), fiscal_year_start range, and duplicate combo-Date rows.
Contributor
There was a problem hiding this comment.
Pull request overview
Plan (review focus):
- Verify new tests match current
check_input_data()validation branches and messages. - Check that “positive” tests isolate the intended behavior and won’t fail for unrelated future validations.
- Ensure changes are test-only and consistent with existing test conventions.
This PR adds regression coverage around R/input_checks.R::check_input_data() by asserting the current user-facing validation errors across previously untested branches, helping prevent accidental message/behavior regressions without changing runtime behavior.
Changes:
- Added
testthatcases for missing required columns (combo vars, target, xregs) and invalid target type. - Added coverage for
Datecolumn presence/type, month day-of-month consistency, fiscal year start range, and duplicate combo–Date rows. - Added a “week” positive test intended to confirm the month day-of-month check is bypassed for
date_type = "week".
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Evidence
R/input_checks.R::check_input_data()contains a number of detailed, user-facingvalidation error messages (several recently enhanced) that had no regression
coverage in
tests/testthat/test-input_checks.R. A baseline reproducer confirmedthe exact current message for each branch, so the behavior is well-defined but
unguarded against accidental regressions.
Change
Add focused
testthatcoverage for the previously untestedcheck_input_datavalidation branches. No source behavior is changed — this is test-only:
DatecolumnDatecolumnmonthdata, plus a positive test thatdate_type = "week"bypasses that checkfiscal_year_startoutside 1–12DaterowsDependency decision
No new dependency added. Existing capabilities were sufficient:
testthat(already in
Suggests) and base R. This is test-only coverage, so the dependencyladder resolves at step 2 (existing declared dependencies).
Validation
Each category was run in its own process with
Rscript -e "devtools::test(filter = '<filter>', stop_on_failure = TRUE)",affected category first:
^input_checks$^(input_checks|hash_data|prep_data|clamp_negative_target|undifference)$^(forecast_time_series|multistep|hierarchical)$^(best_models|final_models|summarize_models)$^(agent|finalize_run|load_run_results)$^(foundation-model-credentials|chronos|timegpt|timesfm)$All 18
test-*.Rfiles map exactly to the five configured categories; nounclassified files. No roxygen/source changes, so
devtools::document()was notrequired.
Rscript -e "devtools::check(args = '--no-tests')"completed all package checksbut errored only at vignette re-building (
'xfun::attr()' is deprecated). This isa pre-existing environment/toolchain limitation: the identical command on clean
mainfails at the same vignette step with the same error, so it is unrelated tothis test-only change. All focused and category tests pass.
Risk and limitations
Blast radius 1 (test-only; no runtime or public-API change). These tests lock in
the current user-facing validation messages, so intentional future message edits
must update the corresponding assertions. The only unavailable gate is the
pre-existing vignette build error noted above.
Automation metadata
53908f7d75ea2f37ccf269afada8bc62e99bc5c782eda7658e3d9c96cbfdb0c1b913ca8671b273281591527a139b1ccb2bdd6c77