refactor(tests): update integration test setup#990
Draft
Bai-Li-NOAA wants to merge 25 commits intodevfrom
Draft
Conversation
* set verbosity to `quiet` in `tests/testthat.R` to suppress test output when running `devtools::test()` locally. * set verbosity to `quiet` in `tests/testthat/helper-aaa-quiet-test-output.R` to suppress output when running `testthat::test_package()` locally. This mirrors the "Show testthat output" (`test_check()`) step in the `call-r-cmd-check` GitHub Action workflow. * set `silent = TRUE` in `TMB::MakeADFun()` calls to suppress TMB output during tests. * use `suppressMessages()` to silence output from fims_frame tests and testthat template tests. * don't use `Rcerr` to print a message to the console when the ParameterVector index is out of range, since `throw` already handles the error output.
Bumps [actions/first-interaction](https://github.com/actions/first-interaction) from 1 to 2. - [Release notes](https://github.com/actions/first-interaction/releases) - [Commits](actions/first-interaction@v1...v2) --- updated-dependencies: - dependency-name: actions/first-interaction dependency-version: '2' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
* document(check_fims.R): Add examples of strings passed to ... in
FIMS:::setup_and_run_gtest()
* refactor(fimsfit.R):
* Remove is.FIMSFits() function
* Remove "total" from number_of_parameters; only print it
as the sum of fixed_effects + random_effects
* document(fimsfit.R): Add comment referencing sdmTMB issue #455 to
evaluate whether report should always use last.par.best
* document(initialize_modules.R): Clarify that dims field is necessary
to track dimensions for multivariate input (e.g., MVNORM) and may be
needed for other distributions
* update(data1.R): Remove check for CSV readability in R
* update(helper-integration-tests-setup-function.R): Replace 1:x with
seq_along() in for loop for safer indexing
Co-authored-by: Andrea-Havron-NOAA <Andrea-Havron-NOAA@users.noreply.github.com>
Co-authored-by: k-doering-NOAA <k-doering-NOAA@users.noreply.github.com>
Co-authored-by: kellijohnson-NOAA <kellijohnson-NOAA@users.noreply.github.com>
Co-authored-by: msupernaw <msupernaw@users.noreply.github.com>
Co-authored-by: nathanvaughan-NOAA <nathanvaughan-NOAA@users.noreply.github.com>
Co-authored-by: peterkuriyama-NOAA <peterkuriyama-NOAA@users.noreply.github.com>
* add README.md under the /tests directory as a quick reference for testing. It includes instructions on adding a new test, run tests, and debug tests. * add a C++ test template to inst/templates. * add a new R function `use_gtest_template()` to create a C++ test file using the template and register the test in `tests/gtest/CMakeLists.txt`. * add R unit tests for `use_gtest_template()`. * add a C++ test `tests/gtest/test_FIMSJson_JsonParser_WriteToFile.cpp` to verify error handling instructions outlined in the C++ test template. * update LogisticSelectivity C++ test to use the new template. * fix spelling errors and update WORDLIST. * Place `Rcpp::loadModule(module = "fims", what = TRUE)` inside the `.onLoad` function has been observed to fix R session crashes when running `devtools::load_all()` followed by `devtools::test()` in the package development workflow.
Bumps [actions/checkout](https://github.com/actions/checkout) from 4 to 5. - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v4...v5) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '5' dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
* chore(devcontainer): add lldb for debugging support * chore(readme): add FIMS local dev setup cheat sheet * chore(spell-check): check additional files and update WORDLIST
Close #910 Co-authored-by: Abhinav Kumar <abhinavkumar130503@gmail.com>
Much of the content was moved to CONTRIBUTING.md but some was also moved to the README. The documentation vignette was removed because it mainly contained links to resources that we are trying to consolidate into more intuitive locations. `check_fims()` was deprecated and the list of items was moved to CONTRIBUTING.md. Adds information from collaborative workflow to the README in the tests folder. Close #947
* add create_default_configurations() to set up configurations based on data * update create_default_*() to use output from create() for parameter specification and it returns a tibble * update initialize_*() to accept a tibble of parameters as input * update vignettes and tests * change "age" and "length" types to "age_comp" and "length_comp" in data1
* add R tests for `create_default_configurations()`. * add error-handling tests for `create_default_parameters()` to cover cases where no parameters are available for optimization. Address issue#552. * add snapshot tests for the outputs of both `create_default_configurations()` and `create_default_parameters()`. Snapshots are saved as CSV files.
Update Doxygen destination repository and folder to fims website instead of a separate repository.
Contributor
Instructions for code reviewerHello reviewer, thanks for taking the time to review this PR!
Checklist
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #990 +/- ##
==========================================
- Coverage 80.26% 75.30% -4.96%
==========================================
Files 51 83 +32
Lines 1941 8035 +6094
Branches 461 311 -150
==========================================
+ Hits 1558 6051 +4493
- Misses 262 1978 +1716
+ Partials 121 6 -115 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Bai-Li-NOAA
force-pushed
the
dev-refactor-prepare-test-data
branch
from
3c98c29 to
fd35115
Compare
- Create helper functions (`load_*()`) to load or create data for a FIMS model run, or run a FIMS model with wrappers to save the fit output. These functions check if the corresponding `.RDS` file exists; if not, they generate the data or run the model and save the output to an `.RDS` file. If the file exists, they read and return the object from the `.RDS` file. This ensures that test data is generated only once and is available for subsequent test runs, speeding up the process. - Add error handling (e.g., `tryCatch()`) in data loading functions to provide clearer messages when file operations fail. - Update README to document new helper functions for loading data and running FIMS with wrappers. - Rename `test-integration-caa-mle.R` to `test-integration-caa-mle-without-wrappers.R` so it can be filtered when running tests (`devtools::test(filter = "xxx")`), speeding up single-test execution. - Update snapshot tests outputs from `.md` files to `.csv` files. - Replace nan with -999 in JSON files. Thanks to @msupernaw for the fix to issue #940.
Bai-Li-NOAA
force-pushed
the
dev-refactor-prepare-test-data
branch
from
fd35115 to
786a48f
Compare
kellijohnson-NOAA
force-pushed
the
dev
branch
from
4a24251 to
51adea8
Compare
kellijohnson-NOAA
force-pushed
the
dev
branch
3 times, most recently
from
db3ddf7 to
e0be2a1
Compare
13 tasks
kellijohnson-NOAA
force-pushed
the
dev
branch
from
afee2ed to
1276b24
Compare
kellijohnson-NOAA
force-pushed
the
dev
branch
2 times, most recently
from
b3d15a2 to
343c111
Compare
kellijohnson-NOAA
force-pushed
the
dev
branch
2 times, most recently
from
3bf9e4f to
de9a41e
Compare
Contributor
|
@Bai-Li-NOAA I just wanted to note that I think we can hold off on this until after the CIE review. I do not want you to have to rebase this over and over again. So, let's just leave it as is and if you can pick it back up in May that would be great. What do you think? |
Collaborator
Author
|
@kellijohnson-NOAA Yes, this will be a big rebase task. I thought we had discussed possibly pausing development before or during the CIE review. I could use that pause to work on the rebase, with the goal of having it be the first to merge after the CIE review. 😉 |
kellijohnson-NOAA
force-pushed
the
dev
branch
from
87a3229 to
e1f5808
Compare
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
3 participants
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.
What is the feature?
prepare_test_data()is inefficient for running individual tests #983 and [feature:] let nan values play through the JSON output without crashing to help with debugging #940How have you implemented the solution?
load_*()) to load or create data for a FIMS model run, or run a FIMS model with wrappers to save the fit output. These functions check if the corresponding.RDSfile exists; if not, they generate the data or run the model and save the output to an.RDSfile. If the file exists, they read and return the object from the.RDSfile. This ensures that test data is generated only once and is available for subsequent test runs, speeding up the process.tryCatch()) in data loading functions to provide clearer messages when file operations fail.test-integration-caa-mle.Rtotest-integration-caa-mle-without-wrappers.Rso it can be filtered when running tests (devtools::test(filter = "xxx")), speeding up single-test execution..mdfiles to.csvfiles.Does the PR impact any other area of the project, maybe another repo?
Task:
[ ] add an integration test where we manually set the fixed-effect _F_s to constant (address issue #852)