-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
Detect nesting it
and pending
at run-time
#7297
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @makenowjust 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I just left a question.
@@ -385,6 +385,11 @@ module Spec | |||
raise ex | |||
end | |||
|
|||
# `NestingSpecError` is also treated. | |||
if ex.is_a?(Spec::NestingSpecError) && klass != Spec::NestingSpecError |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this for?
There was a problem hiding this comment.
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.
Just asking: Is this really worth the added complexity? |
@straight-shoota At least we have an issue #7192 and this is the simplest way to fix it. |
Currently nesting `it`/`pending` doesn't break `crystal spec -v` output.
Now, nesting Example: require "spec"
it "foo" do
it "bar" do
end
end
it "baz" do
pending
end Old (broken) output: New (fixed) output: |
@@ -166,6 +166,19 @@ module Spec | |||
def matches?(pattern, line, locations) | |||
false | |||
end | |||
|
|||
@@spec_nesting = false |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes
There was a problem hiding this comment.
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
Fix #7192
Close #7207