Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Get rid of assertThrow() in soltest #15906

Merged
merged 1 commit into from
Mar 3, 2025
Merged

Conversation

cameel
Copy link
Member

@cameel cameel commented Mar 1, 2025

A tiny bit of cleanup extracted from #15905. I removed a couple of assertThrow()s from the code I was touching and then noticed that there aren't that many left in tests overall so I can just do them all.

For context, assertThrow() is a weird legacy macro that is named as if it was an assert but instead of reporting an ICE, throws the user-supplied exception, which makes it a validation (when used properly). And of course it's often abused for things that are not validations. There's no reason to use it at all and it should eventually be replaced by the more self-explanatory solAssert()/solRequire()/solUnimplemented(), depending on what it's actually used for.

@cameel cameel requested review from clonker and matheusaaguiar March 1, 2025 18:30
@cameel cameel self-assigned this Mar 1, 2025
@cameel cameel force-pushed the get-rid-of-assert-throw-in-tests branch from f6aee19 to 7ffb394 Compare March 3, 2025 16:09
@cameel cameel enabled auto-merge March 3, 2025 16:09
@cameel cameel merged commit d180475 into develop Mar 3, 2025
74 checks passed
@cameel cameel deleted the get-rid-of-assert-throw-in-tests branch March 3, 2025 16:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants