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

Detect nesting it and pending at run-time #7297

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion spec/std/array_spec.cr
Expand Up @@ -1096,7 +1096,7 @@ describe "Array" do

describe "to_s" do
it "does to_s" do
it { [1, 2, 3].to_s.should eq("[1, 2, 3]") }
[1, 2, 3].to_s.should eq("[1, 2, 3]")
end

it "does with recursive" do
Expand Down
8 changes: 4 additions & 4 deletions spec/std/char_spec.cr
Expand Up @@ -373,10 +373,10 @@ describe "Char" do
end

it "does number?" do
it { '1'.number?.should be_true }
it { '٠'.number?.should be_true }
it { '٢'.number?.should be_true }
it { 'a'.number?.should be_false }
'1'.number?.should be_true
'٠'.number?.should be_true
'٢'.number?.should be_true
'a'.number?.should be_false
end

it "does ascii_control?" do
Expand Down
2 changes: 1 addition & 1 deletion spec/std/deque_spec.cr
Expand Up @@ -545,7 +545,7 @@ describe "Deque" do

describe "to_s" do
it "does to_s" do
it { Deque{1, 2, 3}.to_s.should eq("Deque{1, 2, 3}") }
Deque{1, 2, 3}.to_s.should eq("Deque{1, 2, 3}")
end

it "does with recursive" do
Expand Down
2 changes: 1 addition & 1 deletion spec/std/float_spec.cr
Expand Up @@ -14,7 +14,7 @@ describe "Float" do

describe "%" do
it "uses modulo behavior, not remainder behavior" do
it { ((-11.5) % 4.0).should eq(0.5) }
((-11.5) % 4.0).should eq(0.5)
end
end

Expand Down
12 changes: 12 additions & 0 deletions spec/std/spec_spec.cr
Expand Up @@ -114,6 +114,18 @@ describe "Spec matchers" do
true.should be_truthy
end
end

it "detects a nesting `it`" do
ex = expect_raises(Spec::NestingSpecError) { it { } }
ex.message.should eq "can't nest `it` or `pending`"
ex.file.should eq __FILE__
end

it "detects a nesting `pending`" do
ex = expect_raises(Spec::NestingSpecError) { pending }
ex.message.should eq "can't nest `it` or `pending`"
ex.file.should eq __FILE__
end
end

describe "Spec" do
Expand Down
19 changes: 16 additions & 3 deletions src/spec/context.cr
Expand Up @@ -81,21 +81,21 @@ module Spec
puts
puts "#{(i + 1).to_s.rjust(3, ' ')}) #{fail.description}"

if ex.is_a?(AssertionFailed)
if ex.is_a?(SpecError)
source_line = Spec.read_line(ex.file, ex.line)
if source_line
puts Spec.color(" Failure/Error: #{source_line.strip}", :error)
end
end
puts

message = ex.is_a?(AssertionFailed) ? ex.to_s : ex.inspect_with_backtrace
message = ex.is_a?(SpecError) ? ex.to_s : ex.inspect_with_backtrace
message.split('\n').each do |line|
print " "
puts Spec.color(line, :error)
end

if ex.is_a?(AssertionFailed)
if ex.is_a?(SpecError)
puts
puts Spec.color(" # #{Spec.relative_file(ex.file)}:#{ex.line}", :comment)
end
Expand Down Expand Up @@ -166,6 +166,19 @@ module Spec
def matches?(pattern, line, locations)
false
end

@@spec_nesting = false
Copy link
Contributor

@Fryguy Fryguy Jan 18, 2019

Choose a reason for hiding this comment

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

If we were to ever get to parallel test execution, would this class variable cause a problem?

Copy link
Member

Choose a reason for hiding this comment

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

Yes

Copy link
Member

Choose a reason for hiding this comment

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

Oh, actually, I was replying generically that class variables are to be used carefully in a parallel context. Not sure about this case, though


def self.check_nesting_spec(file, line, &block)
raise NestingSpecError.new("can't nest `it` or `pending`", file, line) if @@spec_nesting

@@spec_nesting = true
begin
yield
ensure
@@spec_nesting = false
end
end
end

# :nodoc:
Expand Down
10 changes: 9 additions & 1 deletion src/spec/dsl.cr
Expand Up @@ -38,7 +38,7 @@ module Spec
end

# :nodoc:
class AssertionFailed < Exception
class SpecError < Exception
getter file : String
getter line : Int32

Expand All @@ -47,6 +47,14 @@ module Spec
end
end

# :nodoc:
class AssertionFailed < SpecError
end

# :nodoc:
class NestingSpecError < SpecError
end

@@aborted = false

# :nodoc:
Expand Down
5 changes: 5 additions & 0 deletions src/spec/expectations.cr
Expand Up @@ -385,6 +385,11 @@ module Spec
raise ex
end

# `NestingSpecError` is treated as the same above.
if ex.is_a?(Spec::NestingSpecError) && klass != Spec::NestingSpecError
Copy link
Member

Choose a reason for hiding this comment

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

What is this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See full code:

    def expect_raises(klass : T.class, message = nil, file = __FILE__, line = __LINE__) forall T
      yield
    rescue ex : T
      # We usually bubble Spec::AssertaionFailed, unless this is the expected exception
      if ex.is_a?(Spec::AssertionFailed) && klass != Spec::AssertionFailed
        raise ex
      end

      # `NestingSpecError` is treated as the same above.
      if ex.is_a?(Spec::NestingSpecError) && klass != Spec::NestingSpecError
        raise ex
      end

      # ...
    end

Please tell me more correct sentence which means this if you know.

raise ex
end

ex_to_s = ex.to_s
case message
when Regex
Expand Down
40 changes: 22 additions & 18 deletions src/spec/methods.cr
Expand Up @@ -36,23 +36,25 @@ module Spec::Methods
#
# It is usually used inside a `#describe` or `#context` section.
def it(description = "assert", file = __FILE__, line = __LINE__, end_line = __END_LINE__, &block)
return unless Spec.matches?(description, file, line, end_line)
Spec::RootContext.check_nesting_spec(file, line) do
return unless Spec.matches?(description, file, line, end_line)

Spec.formatters.each(&.before_example(description))
Spec.formatters.each(&.before_example(description))

start = Time.monotonic
begin
Spec.run_before_each_hooks
block.call
Spec::RootContext.report(:success, description, file, line, Time.monotonic - start)
rescue ex : Spec::AssertionFailed
Spec::RootContext.report(:fail, description, file, line, Time.monotonic - start, ex)
Spec.abort! if Spec.fail_fast?
rescue ex
Spec::RootContext.report(:error, description, file, line, Time.monotonic - start, ex)
Spec.abort! if Spec.fail_fast?
ensure
Spec.run_after_each_hooks
start = Time.monotonic
begin
Spec.run_before_each_hooks
block.call
Spec::RootContext.report(:success, description, file, line, Time.monotonic - start)
rescue ex : Spec::AssertionFailed
Spec::RootContext.report(:fail, description, file, line, Time.monotonic - start, ex)
Spec.abort! if Spec.fail_fast?
rescue ex
Spec::RootContext.report(:error, description, file, line, Time.monotonic - start, ex)
Spec.abort! if Spec.fail_fast?
ensure
Spec.run_after_each_hooks
end
end
end

Expand All @@ -68,11 +70,13 @@ module Spec::Methods
#
# It is usually used inside a `#describe` or `#context` section.
def pending(description = "assert", file = __FILE__, line = __LINE__, end_line = __END_LINE__, &block)
return unless Spec.matches?(description, file, line, end_line)
Spec::RootContext.check_nesting_spec(file, line) do
return unless Spec.matches?(description, file, line, end_line)

Spec.formatters.each(&.before_example(description))
Spec.formatters.each(&.before_example(description))

Spec::RootContext.report(:pending, description, file, line)
Spec::RootContext.report(:pending, description, file, line)
end
end

# Defines a yet-to-be-implemented pending test case
Expand Down