test: disable return false handling#22147
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates Homebrew’s formula test runner (brew test) to treat formula tests that return false as disabled behavior (via Utils::Output.odisabled) rather than merely deprecated behavior (odeprecated), aligning with the direction of removing “return false” handling in formula test blocks.
Changes:
- Switch the warning/error mechanism from
Utils::Output.odeprecatedtoUtils::Output.odisabledwhen a formula test returnsfalse. - Make “returning false from tests” behave as a hard-disabled path (raising a
MethodDeprecatedError).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if formula.run_test(keep_tmp: args.keep_tmp?) == false | ||
| require "utils/output" | ||
| Utils::Output.odeprecated "`return false` in test", "`raise \"<reason for failure>\"`" | ||
| Utils::Output.odisabled "`return false` in test", "`raise \"<reason for failure>\"`" |
There was a problem hiding this comment.
I'd like to handle this in the follow-up PR that removes the proc wrapper. Adding caller: here is tricky because of define_method, so a redesign later will be cleaner.
MikeMcQuaid
left a comment
There was a problem hiding this comment.
Thanks @hyuraku! I think we should wait for the next minor release for this. I know we missed it in the last one 🤦🏻 but I think kicking in new disables out of the normal release cycle will be confusing. Will keep this open!
brew lgtm(style, typechecking and tests) with your changes locally?Upgrade
odeprecatedtoodisabledforreturn falsein formula test blocks.return falsewas deprecated in #21296 (merged 2025-12-21). The deprecation has been live for ~4.5 months.