Conversation
Introduce support for storing JASP example files in tests/testthat/jaspfiles/{library,verified,other} and generate tests named test-{source}-{basename}.R. makeTestsFromExamples() gains a new source argument (defaults depend on overwrite) and when path is provided files are copied into the "other" folder before generating tests. Updated makeTestsFromSingleJASPFile(), generateExampleTestFileContent(), generateExampleTestBlock*() to accept a sourceFolder and to load JASP files via testthat::test_path("jaspfiles", source, file.jasp). Added copyToJaspfiles behavior, overwrite protection/prompt for verified tests, and a summary printer (.printTestGenerationSummary). Also updated documentation/examples and adjusted test discovery (test runner) to include auto-generated source-based tests. These changes centralize example JASP storage, avoid clobbering verified tests by default, and make generated test paths explicit.
There was a problem hiding this comment.
Pull request overview
Refactors makeTestsFromExamples() and related test discovery to support a structured, source-based organization for JASP example files and their generated testthat tests, replacing the previous flat examples/ approach.
Changes:
- Introduces
tests/testthat/jaspfiles/{library,verified,other}/and generatestest-{source}-{name}.Rtest files referencing JASP files viatestthat::test_path("jaspfiles", ...). - Adds a
sourceparameter + overwrite safeguards (defaulting away fromverifiedwhen overwriting, with confirmation). - Updates
testAnalysis()discovery to include source-based tests, and regenerates/updates documentation accordingly.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
R/test-generator.R |
Implements the new source-folder workflow, naming, copy behavior, and summary messaging for generated tests. |
R/test.R |
Updates test-file selection logic to discover and include test-{library,verified,other}-* tests for a given analysis. |
man/makeTestsFromExamples.Rd |
Documents new source parameter, folder structure, and updated examples. |
man/makeTestsFromSingleJASPFile.Rd |
Updates parameter documentation (sourceFolder, copyToJaspfiles) for the internal helper. |
man/testAnalysis.Rd |
Documents the expanded discovery behavior for source-based generated tests. |
man/generateExampleTestFileContent.Rd |
Updates internal docs to reflect source-based test file generation. |
man/generateExampleTestBlock.Rd |
Documents the new sourceFolder argument used to build correct test_path() calls. |
man/generateExampleTestBlockBasic.Rd |
Documents the new sourceFolder argument in the basic fallback test block. |
.github/copilot-instructions.md |
Updates repository conventions/docs for the new folder layout and test naming. |
Comments suppressed due to low confidence (1)
R/test-generator.R:344
makeTestsFromSingleJASPFile()copies the input JASP file totests/testthat/jaspfiles/{sourceFolder}/before checking whether the test file already exists andoverwriteis FALSE. This means an import viapath=can overwrite an existing JASP file even when the corresponding test is skipped. Consider checkingfile.exists(testFilePath) && !overwrite(and returning) before performing the copy, or only copying when the destination file doesn’t already exist.
# Copy JASP file to module's jaspfiles/{sourceFolder}/ if requested
if (copyToJaspfiles) {
destDir <- file.path(module.dir, "tests", "testthat", "jaspfiles", sourceFolder)
if (!dir.exists(destDir)) {
dir.create(destDir, recursive = TRUE)
message(" Created directory: ", destDir)
}
destFile <- file.path(destDir, basename(jaspFile))
file.copy(jaspFile, destFile, overwrite = TRUE)
copiedTo <- destFile
message(" Copied to: ", destFile)
}
# Create tests/testthat directory if needed
testDir <- file.path(module.dir, "tests", "testthat")
if (!dir.exists(testDir)) {
dir.create(testDir, recursive = TRUE)
message(" Created directory: ", testDir)
}
# Determine test file path using "test-{sourceFolder}-Name.R" format
testFileName <- paste0("test-", sourceFolder, "-", sanitizedName, ".R")
testFilePath <- file.path(testDir, testFileName)
# Check if file already exists
if (file.exists(testFilePath) && !overwrite) {
result <- testFilePath
attr(result, "skipped") <- TRUE
attr(result, "copiedTo") <- copiedTo
return(result)
| pattern <- paste0('runAnalysis\\(\\s*["\']', name, '["\']') | ||
| for (ef in sourceFiles) { | ||
| content <- readLines(file.path(testsDir, ef), warn = FALSE) | ||
| if (any(grepl(pattern, content))) | ||
| matchedFiles <- c(matchedFiles, ef) | ||
| } |
There was a problem hiding this comment.
In getTestFilesMatchingName(), the pattern is built by interpolating name directly into a regex and then used with grepl(). If an analysis name ever contains regex metacharacters (e.g., . + (), this can produce false matches or a regex error. Escape name for regex (or otherwise ensure literal matching) before constructing pattern.
| pattern <- paste0('runAnalysis\\(\\s*["\']', name, '["\']') | |
| for (ef in sourceFiles) { | |
| content <- readLines(file.path(testsDir, ef), warn = FALSE) | |
| if (any(grepl(pattern, content))) | |
| matchedFiles <- c(matchedFiles, ef) | |
| } | |
| escapedName <- gsub("([][{}()+*^$|\\\\?.])", "\\\\\\1", name) | |
| pattern <- paste0('runAnalysis\\(\\s*["\']', escapedName, '["\']') | |
| for (ef in sourceFiles) { | |
| content <- readLines(file.path(testsDir, ef), warn = FALSE) | |
| if (any(grepl(pattern, content))) | |
| matchedFiles <- c(matchedFiles, ef) |
| otherDir <- file.path(module.dir, "tests", "testthat", "jaspfiles", "other") | ||
| if (!dir.exists(otherDir)) { | ||
| dir.create(otherDir, recursive = TRUE) |
There was a problem hiding this comment.
otherDir is created to ensure tests/testthat/jaspfiles/other/ exists, but the variable itself is never used afterwards. Consider removing the unused variable (or using it consistently) to reduce noise and avoid linters/CRAN checks flagging it.
| otherDir <- file.path(module.dir, "tests", "testthat", "jaspfiles", "other") | |
| if (!dir.exists(otherDir)) { | |
| dir.create(otherDir, recursive = TRUE) | |
| if (!dir.exists(file.path(module.dir, "tests", "testthat", "jaspfiles", "other"))) { | |
| dir.create(file.path(module.dir, "tests", "testthat", "jaspfiles", "other"), recursive = TRUE) |
| .printTestGenerationSummary(createdFiles, skippedFiles, character(0), paste(source, collapse = ", ")) | ||
| invisible(createdFiles) | ||
| } | ||
|
|
||
| .printTestGenerationSummary <- function(createdFiles, skippedFiles, copiedFiles, sourceLabel) { | ||
| if (length(createdFiles) == 0 && length(skippedFiles) == 0) { | ||
| warning("No test files were created.") | ||
| } else { | ||
| if (length(copiedFiles) > 0) { | ||
| message("\nCopied ", length(copiedFiles), " JASP file(s) to module examples/ folder(s).") | ||
| message("\nCopied ", length(copiedFiles), " JASP file(s) to tests/testthat/jaspfiles/", sourceLabel, "/.") | ||
| } |
There was a problem hiding this comment.
When processing multiple sources, .printTestGenerationSummary() is called with sourceLabel = paste(source, collapse = ", "), but the message formats it as if it were a single folder path (.../jaspfiles/<sourceLabel>/.). This produces misleading output like .../jaspfiles/library, other/. when more than one source is used. Consider adjusting the message to either omit the path formatting for multi-source runs or print separate paths per source.
|
tested on jaspMetaAnalysis successfully |
Refactor
makeTestsFromExamples()with source folder systemSummary
This PR refactors
makeTestsFromExamples()to support a structured three-tier source folder system for JASP example files, replacing the flatexamples/folder approach.Changes
New folder structure
tests/testthat/jaspfiles/{library,verified,other}/test-{source}-{name}.R(e.g.,test-library-CorrelationMatrix.R)testthat::test_path("jaspfiles", "{source}", "file.jasp")New
sourceparameter"library","verified","other"overwrite = FALSEc("library", "other")whenoverwrite = TRUE(protects verified tests)overwrite = TRUEand"verified"is included, prompts user for confirmationpathparameter behaviorpathis provided, JASP files are always copied totests/testthat/jaspfiles/other/sourceargument is ignored in this caseTest discovery
testAnalysis()now automatically discovers all source-based test filestest-library-*,test-verified-*, andtest-other-*files for matching analysesincludeExamplesapproach)Breaking Changes
examples/folder convention is removedtest-example-{name}.R→test-{source}-{name}.Rexamples/→tests/testthat/jaspfiles/{source}/test_path("..", "..", "examples", ...)→test_path("jaspfiles", "{source}", ...)Rationale
The three-tier system provides:
Example Usage
Documentation
.github/copilot-instructions.mdwith new conventionsFiles Changed
R/test-generator.R: Core implementation of source folder systemR/test.R: UpdatedtestAnalysis()andgetTestFilesMatchingName()to discover source-based tests.github/copilot-instructions.md: Updated documentationman/*.Rd: Regenerated documentation files