Skip to content

Deduplicate and reorganize specs for Kernel#raise #1332

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.

Sign up for GitHub

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

Open
trinistr wants to merge 1 commit into
base: master
Choose a base branch
from
+74 −117
Open

Deduplicate and reorganize specs for Kernel#raise #1332

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions core/fiber/raise_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,16 @@
require_relative '../../shared/kernel/raise'

describe "Fiber#raise" do
it "is a public method" do
Fiber.public_instance_methods.should include(:raise)
end

it_behaves_like :kernel_raise, :raise, FiberSpecs::NewFiberToRaise
it_behaves_like :kernel_raise_across_contexts, :raise, FiberSpecs::NewFiberToRaise
end
ruby_version_is "4.0" do
it_behaves_like :kernel_raise_with_cause, :raise, FiberSpecs::NewFiberToRaise
end

describe "Fiber#raise" do
it 'raises RuntimeError by default' do
-> { FiberSpecs::NewFiberToRaise.raise }.should raise_error(RuntimeError)
end
Expand Down Expand Up @@ -126,10 +131,7 @@
end.should raise_error(RuntimeError, "Expected error")
end
end
end


describe "Fiber#raise" do
it "transfers and raises on a transferring fiber" do
root = Fiber.current
fiber = Fiber.new { root.transfer }
Expand Down
104 changes: 17 additions & 87 deletions core/kernel/raise_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,19 @@

describe "Kernel#raise" do
it "is a private method" do
Kernel.should have_private_instance_method(:raise)
Kernel.private_instance_methods.should include(:raise)
end

# Shared specs expect a public #raise method.
public_raiser = Object.new
class << public_raiser
public :raise
end
it_behaves_like :kernel_raise, :raise, public_raiser
it_behaves_like :kernel_raise_with_cause, :raise, public_raiser
end

describe "Kernel#raise with previously rescued exception" do
it "re-raises the previously rescued exception if no exception is specified" do
ScratchPad.record nil

Expand All @@ -28,87 +38,6 @@
ScratchPad.recorded.should be_nil
end

it "accepts a cause keyword argument that sets the cause" do
cause = StandardError.new
-> { raise("error", cause: cause) }.should raise_error(RuntimeError) { |e| e.cause.should == cause }
end

it "accepts a cause keyword argument that overrides the last exception" do
begin
raise "first raise"
rescue => ignored
cause = StandardError.new
-> { raise("error", cause: cause) }.should raise_error(RuntimeError) { |e| e.cause.should == cause }
end
end
Copy link
Member

@andrykonchin andrykonchin Jan 28, 2026

Choose a reason for hiding this comment

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

minor: haven't found a similar test in :kernel_raise_with_cause. Isn't it just being deleted?

Copy link
Contributor Author

@trinistr trinistr Jan 28, 2026

Choose a reason for hiding this comment

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

Seems I had code blindness when reviewing changes. I've added tests for this and the next one. Also added a context "without cause keyword argument", which now houses tests for what happens when cause is not specified.


it "raises an ArgumentError when only cause is given" do
cause = StandardError.new
-> { raise(cause: cause) }.should raise_error(ArgumentError, "only cause is given with no arguments")
end

it "raises an ArgumentError when only cause is given even if it has nil value" do
-> { raise(cause: nil) }.should raise_error(ArgumentError, "only cause is given with no arguments")
end

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
Copy link
Member

@andrykonchin andrykonchin Jan 28, 2026

Choose a reason for hiding this comment

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

minor: similar issue

Copy link
Contributor Author

@trinistr trinistr Jan 28, 2026

Choose a reason for hiding this comment

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

See above comment


it "allows cause equal an exception" do
e = RuntimeError.new("message")
-> { raise e, cause: e }.should raise_error(e)
end

it "doesn't set given cause when it equals an exception" do
e = RuntimeError.new("message")

begin
raise e, cause: e
rescue
end

e.cause.should == nil
end

it "raises ArgumentError when exception is part of the cause chain" do
-> {
begin
raise "Error 1"
rescue => e1
begin
raise "Error 2"
rescue => e2
begin
raise "Error 3"
rescue => e3
raise e1, cause: e3
end
end
end
}.should raise_error(ArgumentError, "circular causes")
end

it "re-raises a rescued exception" do
-> do
begin
raise StandardError, "aaa"
rescue Exception
begin
raise ArgumentError
rescue ArgumentError
end

# should raise StandardError "aaa"
raise
end
end.should raise_error(StandardError, "aaa")
end
Copy link
Member

@andrykonchin andrykonchin Jan 28, 2026

Choose a reason for hiding this comment

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

minor: the same issue

Copy link
Contributor Author

@trinistr trinistr Jan 28, 2026

Choose a reason for hiding this comment

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

This one is covered by existing https://github.com/ruby/spec/pull/1332/changes#diff-866cdb19d113cd7e3798855d9e2bc74a30e2307efae414383ee88629580d80a5R20-R39


it "re-raises a previously rescued exception without overwriting the cause" do
begin
begin
Expand Down Expand Up @@ -283,10 +212,11 @@
end
end

describe "Kernel#raise" do
it_behaves_like :kernel_raise, :raise, Kernel
end

describe "Kernel.raise" do
it "needs to be reviewed for spec completeness"
it "is a public method" do
Kernel.singleton_class.should.public_method_defined?(:raise)
end

it_behaves_like :kernel_raise, :raise, Kernel
it_behaves_like :kernel_raise_with_cause, :raise, Kernel
end
7 changes: 7 additions & 0 deletions core/thread/raise_spec.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,15 @@
require_relative '../../shared/kernel/raise'

describe "Thread#raise" do
it "is a public method" do
Thread.public_instance_methods.should include(:raise)
end

it_behaves_like :kernel_raise, :raise, ThreadSpecs::NewThreadToRaise
it_behaves_like :kernel_raise_across_contexts, :raise, ThreadSpecs::NewThreadToRaise
ruby_version_is "4.0" do
it_behaves_like :kernel_raise_with_cause, :raise, ThreadSpecs::NewThreadToRaise
end

it "ignores dead threads and returns nil" do
t = Thread.new { :dead }
Expand Down
68 changes: 43 additions & 25 deletions shared/kernel/raise.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,35 @@ def e.exception
}
end
end
end

ruby_version_is "4.0" do
it "allows cause keyword argument" do
describe :kernel_raise_with_cause, shared: true do
context "without cause keyword argument" do
it "sets cause to nil when there is no previous exception" do
-> do
@object.raise("error without a cause")
end.should raise_error(RuntimeError, "error without a cause") do |error|
error.cause.should == nil
end
end

it "supports automatic cause chaining from a previous exception" do
-> do
begin
raise StandardError,"first error"
rescue
@object.raise("second error")
end
end.should raise_error(RuntimeError, "second error") do |error|
error.cause.should be_kind_of(StandardError)
error.cause.message.should == "first error"
end
end
end

context "with cause keyword argument" do
it "allows setting exception's cause" do
cause = StandardError.new("original error")
result = nil

-> do
@object.raise("new error", cause: cause)
Expand All @@ -153,6 +177,14 @@ def e.exception
end
end

it "allows setting cause to nil" do
-> do
@object.raise("error without a cause", cause: nil)
end.should raise_error(RuntimeError, "error without a cause") do |error|
error.cause.should == nil
end
end

it "raises an ArgumentError when only cause is given" do
cause = StandardError.new("cause")
-> do
Expand All @@ -166,7 +198,7 @@ def e.exception
end.should raise_error(ArgumentError, "only cause is given with no arguments")
end

it "raises a TypeError when given cause is not an instance of Exception" do
it "raises a TypeError when given cause is not an instance of Exception or nil" do
cause = Object.new
-> do
@object.raise("message", cause: cause)
Expand All @@ -175,7 +207,6 @@ def e.exception

it "doesn't set given cause when it equals the raised exception" do
cause = StandardError.new("cause")
result = nil

-> do
@object.raise(cause, cause: cause)
Expand All @@ -185,18 +216,7 @@ def e.exception
end
end

it "accepts cause equal an exception" do
error = RuntimeError.new("message")
result = nil

-> do
@object.raise(error, cause: error)
end.should raise_error(RuntimeError, "message") do |e|
e.cause.should == nil
end
end

it "rejects circular causes" do
it "raises ArgumentError when cause creates a circular reference" do
-> {
begin
raise "Error 1"
Expand All @@ -216,7 +236,6 @@ def e.exception

it "supports exception class with message and cause" do
cause = StandardError.new("cause message")
result = nil

-> do
@object.raise(ArgumentError, "argument error message", cause: cause)
Expand All @@ -230,7 +249,6 @@ def e.exception
it "supports exception class with message, backtrace and cause" do
cause = StandardError.new("cause message")
backtrace = ["line1", "line2"]
result = nil

-> do
@object.raise(ArgumentError, "argument error message", backtrace, cause: cause)
Expand All @@ -242,21 +260,21 @@ def e.exception
end
end

it "supports automatic cause chaining" do
it "supports cause: exception, overriding previous exception" do
custom_error = StandardError.new("custom error")
-> do
begin
raise "first error"
rescue
# No explicit cause - should chain automatically:
@object.raise("second error")
@object.raise("second error", cause: custom_error)
end
end.should raise_error(RuntimeError, "second error") do |error|
error.cause.should be_kind_of(RuntimeError)
error.cause.message.should == "first error"
error.cause.should be_kind_of(StandardError)
error.cause.message.should == "custom error"
end
end

it "supports cause: nil to prevent automatic cause chaining" do
it "supports cause: nil, discarding previous exception" do
-> do
begin
raise "first error"
Expand Down
Loading