Skip to content

Fix plot fallback snapshots#82

Merged
vandenman merged 3 commits into
jasp-stats:masterfrom
FBartos:fix/plot-fallback-performance
Jun 22, 2026
Merged

Fix plot fallback snapshots#82
vandenman merged 3 commits into
jasp-stats:masterfrom
FBartos:fix/plot-fallback-performance

Conversation

@FBartos

@FBartos FBartos commented May 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes the ggplot visual snapshot fallback lifecycle and the slowdown caused by repeatedly falling back through structural snapshots.

New behavior

  • Visual snapshots remain the primary check.
  • ggplot structural .rds snapshots are maintained as fallback fixtures.
  • Missing structural fixtures are written only when the visual comparison passed, when structural update mode is enabled, or for newly generated visual snapshots.
  • Structural fixtures are not created after a visual mismatch against an existing visual snapshot.
  • If the visual check fails but the structural fallback passes, the test warns with the .new.svg review path and asks the user to accept the visual fixture so future runs do not keep using the slower fallback.
  • Stale .new.svg files are no longer treated as current mismatches.
  • Plot snapshot rendering runs a small opt-out GC step by default to reduce large generated-test slowdown: options(jaspTools.plotSnapshot.gc = FALSE) disables it.

Validation

  • testthat::test_dir(tests/testthat, reporter = summary) passes.
  • git diff --check passes, apart from existing CRLF warnings on Windows.
  • CI-style rcmdcheck::rcmdcheck(args = c(--no-manual, --as-cran, --no-examples), error_on = never) has 0 errors and only the existing allowed warning/notes.
  • Verified against the slow jaspMetaAnalysis forest edge-case file; runtime dropped from the pathological fallback behavior to normal visual-check runtime, with an additional GC-related speedup.

@vandenman could you review this plot snapshot fallback behavior?

@FBartos FBartos force-pushed the fix/plot-fallback-performance branch from 4cc1e3a to 0e95e0b Compare May 29, 2026 06:22
@FBartos

FBartos commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

Added the companion fix for the eager-decoded result-state contract in jaspBase.

Changes here:

  • The fast test plot writer now uses jaspBase's internal decoded-plot materializer when available, so fast replay tests do not persist raw encoded plot objects.
  • moveJaspHtmlToDir() now creates the html output directory before copying assets. This avoids the file.copy(...): more 'from' files than 'to' files failure when running runAnalysis(..., view = TRUE) in a fresh Rscript temp directory.
  • Added focused tests for both paths.

Verified with:

  • test-runAnalysis-fast-test-plots.R
  • test-view.R
  • A MixedModelsGLMM replay from Larks and Owls.jasp with view = TRUE using a stub viewer; returned state contains no encoded JASP column tokens.

Related jaspBase work: jasp-stats/jaspBase#204.

@FBartos

FBartos commented May 29, 2026

Copy link
Copy Markdown
Contributor Author

@vandenman this is ready for your review. The jaspTools side remains orchestration-only; the paired jaspBase provenance fix has been pushed and the focused bridge/replay checks pass locally.

Comment thread R/run.R Outdated
return(invisible(results))
}

useFastTestPlotImages <- function() {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This overwrite of jaspBase writeImageJaspResults should be reverted. The other changes to the snapshots make sense. Note that a lot of slowdown in plots has been fixed by updating a dependency (rlang::hash was extremely slow, since fixed on devel).

Revert the test-time replacement of jaspBase::writeImageJaspResults so runAnalysis exercises the core image writer during tests. Drop the dedicated tests for that removed fast-path while keeping the structural plot snapshot fallback changes intact.
@vandenman vandenman merged commit 55ab6bc into jasp-stats:master Jun 22, 2026
9 checks passed
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.

3 participants