Skip to content

Conversation

@vinistock
Copy link
Member

@vinistock vinistock commented Jul 14, 2025

Motivation

I identified an issue we did not foresee. By requiring our custom reporters through RUBYOPT, we end up causing any subprocesses spawned by the user's code to also require them. This can lead to weird issues.

For example, if the user code raises, then our at_exit hook that checks for crashes will run and send a duplicate finish event to the explorer after we already closed the current run request. Since there's no current run, the explorer then thinks that this is a test being executed from the terminal manually and creates a fresh run with zero results inside.

Note: it is possible that this is the infamous issue where spring prevents us from reporting test execution events, but I'm not 100% confident yet.

Implementation

Instead of trying to sanitize RUBYOPT, which can become tricky very quickly, we should just move to using the -r command line argument for Ruby. That simply requires the reporter without cascading down to subprocesses and does what we want.

Note: this is a breaking change because the Rails add-on will now need to add the same require to its command.

@vinistock vinistock self-assigned this Jul 14, 2025
Copy link
Member Author


How to use the Graphite Merge Queue

Add the label graphite-merge to this PR to add it to the merge queue.

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

This stack of pull requests is managed by Graphite. Learn more about stacking.

@vinistock vinistock added breaking-change Non-backward compatible change server This pull request should be included in the server gem's release notes labels Jul 14, 2025 — with Graphite App
@vinistock vinistock added the vscode This pull request should be included in the VS Code extension's release notes label Jul 14, 2025 — with Graphite App
@vinistock vinistock marked this pull request as ready for review July 14, 2025 18:43
@vinistock vinistock requested a review from a team as a code owner July 14, 2025 18:43
@vinistock vinistock merged commit 7c1b294 into main Jul 15, 2025
39 checks passed
@vinistock vinistock deleted the 07-14-prevent_child_processes_from_requiring_the_reporter_accidentally branch July 15, 2025 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-change Non-backward compatible change server This pull request should be included in the server gem's release notes vscode This pull request should be included in the VS Code extension's release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants