Skip to content

Comments

Fix/testable slurm#22

Merged
Alon-Alexander merged 3 commits intomainfrom
fix/testable_slurm
Feb 13, 2026
Merged

Fix/testable slurm#22
Alon-Alexander merged 3 commits intomainfrom
fix/testable_slurm

Conversation

@Alon-Alexander
Copy link
Owner

@Alon-Alexander Alon-Alexander commented Feb 13, 2026

PR Type

Bug fix, Tests, Enhancement


Description

  • Fix SLURM tests to pass under R CMD check by using full Rscript path

  • Improve mock SLURM job runner to return "PENDING" status correctly

  • Expand test coverage across SLURM, analysis, project, parse_inputs, and file writing modules

  • Bump version to 0.1.12 and update documentation


Diagram Walkthrough

flowchart LR
  A["SLURM Mock Runner"] -->|"Use full Rscript path"| B["R CMD Check Compatible"]
  C["Mock squeue"] -->|"Return PENDING status"| D["Correct Job Status Detection"]
  E["Test Suite"] -->|"Add 60+ new tests"| F["Improved Coverage"]
  G["Version 0.1.12"] -->|"Update docs"| H["Documentation Updated"]
Loading

File Walkthrough

Relevant files
Bug fix, tests
1 files
test_slurm.R
Fix SLURM mock runner and expand test coverage                     
+178/-14
Tests
4 files
test_parse_inputs.R
Add comprehensive input validation and error handling tests
+122/-0 
test_file_reading.R
Add tests for RData object_names parameter                             
+25/-0   
test_analysis.R
Add test for standalone analysis project access error       
+20/-0   
test_project_creation.R
Add test for .gitignore copy failure handling                       
+22/-0   
Configuration changes
1 files
DESCRIPTION
Bump version from 0.1.11 to 0.1.12                                             
+1/-1     
Documentation
25 files
NEWS.md
Add changelog entry for version 0.1.12                                     
+11/-0   
index.md
Add detailed release notes for 0.1.12                                       
+20/-0   
index.html
Update HTML changelog with 0.1.12 release notes                   
+13/-1   
authors.md
Update version reference to 0.1.12                                             
+2/-2     
authors.html
Update HTML citation with version 0.1.12                                 
+3/-3     
getting-started.html
Update version and regenerate example output                         
+13/-13 
getting-started.md
Update example output paths in markdown                                   
+11/-11 
file-formats.html
Update version and environment object references                 
+3/-3     
file-formats.md
Update environment object reference in examples                   
+1/-1     
input-definitions.html
Update version and build date                                                       
+2/-2     
slurm-integration.html
Update version and build date                                                       
+2/-2     
PMAnalysis.html
Update version and example output paths                                   
+6/-6     
PMAnalysis.md
Update example output paths in markdown                                   
+5/-5     
PMProject.html
Update version and example output paths                                   
+6/-6     
PMProject.md
Update example output paths in markdown                                   
+5/-5     
PMSlurmRun.html
Update version number                                                                       
+1/-1     
pm_infer_analysis.html
Update version and example output paths                                   
+7/-7     
pm_infer_analysis.md
Update example output paths in markdown                                   
+6/-6     
pm_project.html
Update version and example output paths                                   
+2/-2     
pm_project.md
Update example output paths in markdown                                   
+1/-1     
pm_create_project.html
Update version and example output paths                                   
+2/-2     
pm_create_project.md
Update example output paths in markdown                                   
+1/-1     
pm_write_file.html
Update version number                                                                       
+1/-1     
pm_read_file.html
Update version number                                                                       
+1/-1     
pkgdown.yml
Update build timestamp                                                                     
+1/-1     
Additional files
22 files
404.html +1/-1     
LICENSE.html +1/-1     
index.html +1/-1     
index.html +1/-1     
PMData.html +1/-1     
dot-cancel_slurm_job.html +1/-1     
dot-check_missing_entries.html +1/-1     
dot-check_missing_files.html +1/-1     
dot-check_slurm_job_done.html +1/-1     
dot-check_slurm_job_success.html +1/-1     
dot-extract_input_ids.html +1/-1     
dot-find_code_folder_name.html +1/-1     
dot-format_validation_error.html +1/-1     
dot-get_slurm_job_error.html +1/-1     
dot-submit_slurm_job_with_env.html +1/-1     
dot-validate_input_fields.html +1/-1     
dot-validate_input_files.html +1/-1     
dot-validate_inputs_schema.html +1/-1     
dot-validate_local_inputs_schema.html +1/-1     
index.html +1/-1     
is_slurm_available.html +1/-1     
search.json +1/-1     

@Alon-Alexander Alon-Alexander self-assigned this Feb 13, 2026
@qodo-free-for-open-source-projects

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
DRY out Rscript path resolution

Extract the duplicated logic for finding the Rscript path into a helper function
to reduce code duplication and improve maintainability.

tests/testthat/test_slurm.R [231-237]

-# R CMD check requires a full path to Rscript; never use bare "Rscript"
-rscript_path <- base::file.path(base::R.home(), "bin", "Rscript")
-if (!base::file.exists(rscript_path)) {
-  alt <- .real_sys_which("Rscript")
-  if (alt != "" && grepl("[/\\\\]", alt)) {
-    rscript_path <- alt
+# at top of file
+get_rscript_path <- function() {
+  path <- file.path(R.home(), "bin", "Rscript")
+  if (!file.exists(path)) {
+    alt <- .real_sys_which("Rscript")
+    if (nzchar(alt) && grepl("[/\\\\]", alt)) path <- alt
   }
+  path
 }
+# later in mock_run_slurm_job
+rscript_path <- get_rscript_path()

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion correctly identifies duplicated code for resolving the Rscript path and proposes a good refactoring to a helper function, which improves maintainability.

Medium
  • More
  • Author self-review: I have reviewed the PR code suggestions, and addressed the relevant ones.

@codecov
Copy link

codecov bot commented Feb 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
see 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@Alon-Alexander Alon-Alexander merged commit 1d76e2c into main Feb 13, 2026
13 checks passed
@Alon-Alexander Alon-Alexander deleted the fix/testable_slurm branch February 13, 2026 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant