Add Object validation in Project #728
Conversation
As a part of #715
As a part of #717
* fix save function * Fix tests * Remove duplicated tests * fix bad merge conflict resolution * Restore populationsFolder to populationCSV * remove PI as dependency * Update project directory structure in presentation * Add prerequisite section for installation * Use latest functions in README + knit * Rename Population Folder ProjectConfiguration property - Property is named populationsFolder - Project template folder contains populations is named PopulationsCSV * styler::style_pkg() * correct object case * Add a test that makes sure that ProjectConfiguration can be customize but throws warning if wront value are provided. * Add NEWS entries + format * devtools::document() * extend project configuration tests +fix typo * Prevent user from modifying scenarios directly * Refactor repeated error messages * fix files names * Improve readme (closes #721) * Update documentation * Remove pksim initialization steps (closes #693) * replace deprecated function * pull from main * Update path to where results are stored now * Remove embeded shiny apps * Update plots test for new project structure * remove/relocate outdated test * update snaps * Rename / Comment out of date tests * remove shiny app launching script * merge from main * devtools::documen() * Revert "Validation object & individual modification validation" (#726) * update snapshot after updating packages --------- Co-authored-by: Felix MIL <[email protected]>
|
I think something went wrong during some merge or PR creation: some files are missing (WarningManager is one of them I think) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## implement-project #728 +/- ##
=====================================================
- Coverage 71.32% 70.37% -0.95%
=====================================================
Files 34 35 +1
Lines 4226 4368 +142
=====================================================
+ Hits 3014 3074 +60
- Misses 1212 1294 +82 ☔ View full report in Codecov by Sentry. |
tests/testthat/test-project.R
Outdated
| test_that("Export aborts with invalid directory in non-interactive mode", { | ||
| p <- testProject() | ||
| # Capture cli output using testthat::expect_message() | ||
| expect_message( |
There was a problem hiding this comment.
use expect_error and fill the regex argument to check for error message
| @@ -185,19 +185,33 @@ Project <- | |||
| #' @description Export the entire project to a JSON file | |||
| #' @param filePath The file path where the JSON will be saved | |||
| #' @return NULL | |||
| exportToJSON = function() { | |||
| exportToJSON = function(interactiveInput = TRUE, dirPath = NULL, fileName = NULL) { | |||
There was a problem hiding this comment.
Also provide an export() function defined like this:
export <- function(project, json = TRUE, xlsx = TRUE){}
This function would call internally project$exportToJSON and in the future project$exportToExcel once we have it.
R/project.R
Outdated
| } | ||
| # Validate directory path | ||
| if (!dir.exists(dirPath)) { | ||
| cli::cli_alert_danger("Invalid directory path. Export aborted.") |
There was a problem hiding this comment.
use cli_abort instead and do not return anything ?
| @@ -126,3 +126,44 @@ test_that("Model is not defined", { | |||
| "MODEL_NOT_FOUND" | |||
| ) | |||
| }) | |||
|
|
|||
|
|
|||
| # Project export test ---------------------------------------------------------- | |||
There was a problem hiding this comment.
Also add tests for import.
R/project.R
Outdated
| # Ensure both parameters are provided if interactiveInput is FALSE | ||
| if (is.null(dirPath) || is.null(fileName)) { | ||
| cli::cli_alert_danger("Both 'dirPath' and 'fileName' must be provided when 'interactiveInput' is FALSE.") | ||
| return(invisible(NULL)) |
There was a problem hiding this comment.
It's not the first time I see functions returning NULL when everything's ok. But could you elaborate why we want to do so ?
We don't usually do it in the rest of the package now for homogeneity concerns I would either: not do it or implement it everywhere (but it requires a very good reason to spend time on this).
Originally: #710