Skip to content

Conversation

@talf301
Copy link

@talf301 talf301 commented Dec 18, 2025

Currently when we use SnapshotTesting.jl we need to maintain a global variable indicating whether or not we're regenerating snapshots, and then manually set this variable back and forth when we make apparently breaking changes. This PR adopts some of the ergonomics from ReferenceTests.jl, getting rid of this global variable in each test suite and instead providing 2 options:

  • An environment variable JULIA_SNAPSHOTTESTS_UPDATE indicating whether or not we force updates (defaults to false)
  • When run in an interactive terminal, the test will behave in an interactive way. If the test fails, the user is shown the diff and asked if this is correct and they would like to update accordingly

We get rid of the regenerate flag from the API, so this is a breaking change and requires a major version bump.

@comnik comnik requested a review from Copilot January 5, 2026 18:53
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR removes the regenerate parameter from the snapshot testing API and replaces it with two more ergonomic update mechanisms: an environment variable (JULIA_SNAPSHOTTESTS_UPDATE) for automated updates and interactive prompting when running in a terminal.

Key changes:

  • Removed the regenerate parameter from test_snapshot function signature
  • Added force_update() to check the JULIA_SNAPSHOTTESTS_UPDATE environment variable
  • Implemented interactive update prompting via input_bool() when tests fail in interactive mode
  • Auto-creation of missing snapshot directories with informational messages

Reviewed changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File Description
src/snapshots.jl Core implementation: removed regenerate parameter, added auto-creation logic, interactive update prompts, and helper functions force_update() and input_bool()
test/test_update_modes.jl New comprehensive test suite covering environment variable behavior, interactive input simulation, auto-creation, and successful snapshot scenarios
test/runtests.jl Added new test file to the test suite execution

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +44 to +45
if isinteractive() || force_update()
if force_update() || input_bool("Replace snapshot with actual result (in $subpath)?")
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

The condition isinteractive() || force_update() will enter the interactive prompt even when force_update() is true. Since force_update() already implies automatic updates, the inner condition on line 45 is redundant. Consider restructuring to handle forced updates separately from interactive prompts to avoid unnecessary prompt display when auto-updating.

Suggested change
if isinteractive() || force_update()
if force_update() || input_bool("Replace snapshot with actual result (in $subpath)?")
if force_update()
rm(expected_path; recursive=true, force=true)
cp(snapshot_dir, expected_path; force=true)
@info "Snapshot updated at $expected_path"
@info "Please run the tests again for any changes to take effect"
elseif isinteractive()
if input_bool("Replace snapshot with actual result (in $subpath)?")

Copilot uses AI. Check for mistakes.
Comment on lines +38 to +48
# Create a temporary file with the input
mktempdir() do tmpdir
input_file = joinpath(tmpdir, "input.txt")
write(input_file, input_string)

open(input_file, "r") do input_io
redirect_stdin(input_io) do
redirect_stdout(devnull) do
SnapshotTesting.input_bool("Test prompt")
end
end
Copy link

Copilot AI Jan 5, 2026

Choose a reason for hiding this comment

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

The test_input helper function creates a temporary directory and file for each test case, which adds unnecessary overhead. Consider using IOBuffer for simpler and more efficient input simulation: redirect_stdin(IOBuffer(input_string)) would eliminate the need for temporary files.

Suggested change
# Create a temporary file with the input
mktempdir() do tmpdir
input_file = joinpath(tmpdir, "input.txt")
write(input_file, input_string)
open(input_file, "r") do input_io
redirect_stdin(input_io) do
redirect_stdout(devnull) do
SnapshotTesting.input_bool("Test prompt")
end
end
# Use an in-memory IOBuffer to simulate stdin
io = IOBuffer(input_string)
return redirect_stdin(io) do
redirect_stdout(devnull) do
SnapshotTesting.input_bool("Test prompt")

Copilot uses AI. Check for mistakes.
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