Skip to content

Replace exits with throws in commands - #379

Merged
katafrakt merged 2 commits into
mainfrom
throw-exit
Jul 31, 2026
Merged

Replace exits with throws in commands#379
katafrakt merged 2 commits into
mainfrom
throw-exit

Conversation

@katafrakt

@katafrakt katafrakt commented Jan 21, 2026

Copy link
Copy Markdown
Contributor

I'm seeing test runs ending prematurely again (see #307) locally. This is my attempt to address this problem more systematically, which happened to use the same approach that Tim suggested in the last comment in the other PR.

@github-actions

github-actions Bot commented Jan 21, 2026

Copy link
Copy Markdown
Contributor

ℹ️ Optional job failed: Ruby jruby

@katafrakt
katafrakt marked this pull request as ready for review January 21, 2026 23:54
Commands calling `exit` directly cause instability in test runs, which
also exit early. This leads to false positives.

There was a mechanism injecting alternative exit mock, but you had to
remember to use it every time. We already fixed all (?) the cases once,
but the problem came back.
@timriley

Copy link
Copy Markdown
Member

This is a great improvement, thank you @katafrakt! I'm very sorry for the delay in my review, but I think we can merge this now, and because it's an internal change only, release it with the next release.

I also took a quick look at the other repos where we add or enhance our CLI commands (like hanami-rspec and hanami-reloader), and it doesn't look like any changes are needed there.

@katafrakt

Copy link
Copy Markdown
Contributor Author

No worries about the delay. This is just to ensure that we are always running all specs, so we don't have accidental false positives (happened to me in the past).

@katafrakt
katafrakt merged commit 8bbf744 into main Jul 31, 2026
11 checks passed
@katafrakt
katafrakt deleted the throw-exit branch July 31, 2026 06:59
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.

2 participants