Skip to content

Conversation

@herwinw
Copy link
Member

@herwinw herwinw commented Dec 22, 2025

This was probably a leftover of a copy-paste, these should check TypeError instead of ArgumentError.

This was probably a leftover of a copy-paste, these should check
TypeError instead of ArgumentError.
@eregon eregon merged commit 20b5c9b into ruby:master Dec 22, 2025
13 checks passed
@herwinw herwinw deleted the kernel_raise_exceptions branch December 22, 2025 10:25

it "doesn't raise an ArgumentError when given cause is nil" do
it "doesn't raise a TypeError when given cause is nil" do
-> { raise "message", cause: nil }.should raise_error(RuntimeError, "message")
Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, shouldn't this be a TypeError here then?

Copy link
Member

Choose a reason for hiding this comment

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

doesn't raise a TypeError, i.e. it succeeds but this is raise so succeeding means throwing an exception

Copy link
Contributor

Choose a reason for hiding this comment

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

Oops, somehow completely blinded the "doesn't" part. Maybe the description would be better as "raises RuntimeError".

Copy link
Member Author

Choose a reason for hiding this comment

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

It was written to contrast the test above it:

it "raises a TypeError when given cause is not an instance of Exception" do
  -> { raise "message", cause: Object.new }.should raise_error(TypeError, "exception object expected")
end

it "doesn't raise a TypeError when given cause is nil" do
  -> { raise "message", cause: nil }.should raise_error(RuntimeError, "message")
end

We're testing (speccing?) the different types that can be given as the cause argument. An object that is not nil and not an Exception is not allowed, so the cause argument causes the exception.
In the second test, we're passing a nil as cause argument, this is accepted and ignored, and now the raised exception is what we explicitly do with raise.

So both cases throw an exception, but it's for very different reasons, and the second one is the happy path.

Maybe something like "it accepts nil as a value for cause is accepted and ignored" would be a better description. And to put the icing on the cake, we should probably add a block to raise_error and verify e.cause.should be_nil

Copy link
Member

@eregon eregon Jan 5, 2026

Choose a reason for hiding this comment

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

A describe "the cause: argument" do would also help clarity & structure, and make it clearer those examples are related.

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.

3 participants