Skip to content

BridgeJS: show unified diff on snapshot mismatch#600

Merged
kateinoigakukun merged 1 commit intomainfrom
katei/956b-bridgejs-update
Feb 8, 2026
Merged

BridgeJS: show unified diff on snapshot mismatch#600
kateinoigakukun merged 1 commit intomainfrom
katei/956b-bridgejs-update

Conversation

@kateinoigakukun
Copy link
Member

Motivation

Snapshot failures only reported file paths, making it slow to see what changed.

Overview

Enhance BridgeJS snapshot assertion to write the .actual file, run diff -u, and include the unified diff in the expectation message.

Copy link
Contributor

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

Improves BridgeJS snapshot test failure diagnostics by emitting an .actual file and including a unified diff in the test failure message to make mismatches easier to inspect.

Changes:

  • Write <snapshot>.actual on mismatch and include a diff -u unified diff in the expectation message.
  • Add a helper to run diff via Process and capture its output.

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

Comment on lines +55 to +69
let process = Process()
process.executableURL = URL(fileURLWithPath: "/usr/bin/env")
process.arguments = ["diff", "-u", expectedPath, actualPath]
let output = Pipe()
process.standardOutput = output
process.standardError = Pipe()

do {
try process.run()
} catch {
return nil
}
process.waitUntilExit()

let data = output.fileHandleForReading.readDataToEndOfFile()
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

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

unifiedDiff runs diff with Pipe() output but waits for the process to exit before reading. If the unified diff is large enough to fill the pipe buffer, the child process can block on write and waitUntilExit() can hang the test run. Consider draining stdout/stderr while the process runs (e.g., readabilityHandler like invokeTS2Swift does) or redirecting output to a temporary file and reading it after exit.

Copilot uses AI. Check for mistakes.
Comment on lines +58 to +67
let output = Pipe()
process.standardOutput = output
process.standardError = Pipe()

do {
try process.run()
} catch {
return nil
}
process.waitUntilExit()
Copy link

Copilot AI Feb 8, 2026

Choose a reason for hiding this comment

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

process.standardError is set to a Pipe() but never read. This can both drop useful error output (e.g., diff errors) and has the same pipe-buffer deadlock risk as stdout. Either read stderr as well, or redirect stderr to stdout / a file, and consider checking terminationStatus (0=same, 1=different, 2=error) to decide what to return.

Copilot uses AI. Check for mistakes.
@kateinoigakukun kateinoigakukun merged commit 180c010 into main Feb 8, 2026
16 checks passed
@kateinoigakukun kateinoigakukun deleted the katei/956b-bridgejs-update branch February 8, 2026 03:44
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