Include error count for errors occurring outside examples in summary #2351

JonRowe · 2016-11-15T01:17:57Z

Improve our handling of errors which occur in suite hooks by displaying a count of errors occurred in the summary.

As suggested in #2329

[skip ci]

myronmarston

IMO, the x errors occurred output is confusing. Some test runners (such as minitest and test-unit) differentiate between "failures" (generally an assertion/expectation failure) and "errors" (any other kind of error raised in a test). We've never differentiated that in our output like those runners do, but we do use failure vs error terminology with aggregate_failures:

https://github.com/rspec/rspec-expectations/blob/40b61c44d7ce36bf59dd09c0d0094bb19a1c1f99/spec/rspec/expectations/failure_aggregator_spec.rb#L295

I think there are two ways we can go with this:

Come up with some more specific, accurate language (such as x errors occurred while loading the suite)
Keep the more generic "errors" language but do things like the other test runners and include any non-expectation failure in that count

If we go with the first option I think we should also not include this in the output if we have no errors to report. After all, getting errors at spec file load time is a rarity, and most of the time reporting 0 errors occurred is just noise.

Thoughts?

myronmarston · 2016-11-15T04:36:21Z

lib/rspec/core/notifications.rb

@@ -281,8 +281,10 @@ def fully_formatted
    # @attr pending_examples [Array<RSpec::Core::Example>] the pending examples
    # @attr load_time [Float] the number of seconds taken to boot RSpec
    #                         and load the spec files
+    # @attr errors [Integer] the number of errors that have occurred processing
+    #                        the spec suite


The name errors is a bit mis-leading -- usually I'd expect a field called errors to be an array of errors but here it's just a count. And it's not a count of all errors -- it's a count only of some errors.

The doc still says errors...

JonRowe · 2016-11-15T04:51:33Z

Keep the more generic "errors" language but do things like the other test runners and include any non-expectation failure in that count

It does that, it hooks into the reporter with a method named after exactly that, these are just the only non expectation failure we currently have?

myronmarston · 2016-11-15T05:01:46Z

It does that, it hooks into the reporter with a method named after exactly that, these are just the only non expectation failure we currently have?

No, the reporter method name is notify_non_example_exception which is for exceptions that happen outside of an example, regardless of if they are expectation failures or some other kind of error.

The failures vs errors distinction I was talking about has to do with the type of exception -- e.g. expect(1).to eq 2 raises an exception categorized as a failure because it is an expectation that failed (e.g. something you are specifically checking for in a spec) where as any other kind of exception is categorized as an error.

JonRowe · 2016-11-15T05:38:45Z

We count an error during an example as a failure, I'm happy to change the wording to n errors occurred outside of examples if thats better?

myronmarston · 2016-11-15T05:40:19Z

That is better.

JonRowe · 2016-11-15T05:55:19Z

Done!

myronmarston

Left a couple more comments...

myronmarston · 2016-11-15T06:44:33Z

lib/rspec/core/notifications.rb

@@ -281,8 +281,10 @@ def fully_formatted
    # @attr pending_examples [Array<RSpec::Core::Example>] the pending examples
    # @attr load_time [Float] the number of seconds taken to boot RSpec
    #                         and load the spec files
+    # @attr errors [Integer] the number of errors that have occurred processing
+    #                        the spec suite


The doc still says errors...

myronmarston · 2016-11-15T06:45:30Z

lib/rspec/core/reporter.rb

@@ -18,6 +18,7 @@ def initialize(configuration)
      @failed_examples = []
      @pending_examples = []
      @duration = @start = @load_time = nil
+      @errors = 0


How about calling this non_example_exception_count?

JonRowe · 2016-11-15T08:38:07Z

Updated again

myronmarston · 2016-11-15T15:10:27Z

Merge away.

yujinakayama · 2017-05-04T01:52:37Z

Shouldn't the 0 examples, 0 failures, 3 errors occurred outside of examples line be red when there're errors outside of examples?

Follow-up to #2351.

[skip ci]

Include error count for errors occurring outside examples in summary

Follow-up to rspec#2351.

JonRowe added 2 commits November 15, 2016 11:07

include error count for errors occurring outside examples in summary

e1a4d50

changelog for #2351

c680951

[skip ci]

myronmarston suggested changes Nov 15, 2016

View reviewed changes

reword errors occurred message

81cdb9f

myronmarston suggested changes Nov 15, 2016

View reviewed changes

rename error count variable

c03c0f2

JonRowe force-pushed the errors_occured branch from c028968 to c03c0f2 Compare November 15, 2016 08:28

myronmarston approved these changes Nov 15, 2016

View reviewed changes

JonRowe merged commit 0c8db72 into master Nov 15, 2016

JonRowe deleted the errors_occured branch November 15, 2016 21:58

yujinakayama added a commit that referenced this pull request May 4, 2017

Turn summary line red when there's any error occured outside of examples

c0760ac

Follow-up to #2351.

yujinakayama mentioned this pull request May 4, 2017

Turn summary line red when there's any error occured outside of examples #2421

Merged

MatheusRich pushed a commit to MatheusRich/rspec-core that referenced this pull request Oct 30, 2020

changelog for rspec#2351

11a8bfd

[skip ci]

MatheusRich pushed a commit to MatheusRich/rspec-core that referenced this pull request Oct 30, 2020

Merge pull request rspec#2351 from rspec/errors_occured

97feeb4

Include error count for errors occurring outside examples in summary

MatheusRich pushed a commit to MatheusRich/rspec-core that referenced this pull request Oct 30, 2020

Turn summary line red when there's any error occured outside of examples

205de85

Follow-up to rspec#2351.

Uh oh!

Include error count for errors occurring outside examples in summary #2351

Include error count for errors occurring outside examples in summary #2351

Uh oh!

Conversation

JonRowe commented Nov 15, 2016

Uh oh!

myronmarston left a comment

Choose a reason for hiding this comment

Uh oh!

myronmarston Nov 15, 2016

Choose a reason for hiding this comment

Uh oh!

JonRowe Nov 15, 2016

Choose a reason for hiding this comment

Uh oh!

myronmarston Nov 15, 2016

Choose a reason for hiding this comment

Uh oh!

JonRowe commented Nov 15, 2016

Uh oh!

myronmarston commented Nov 15, 2016

Uh oh!

JonRowe commented Nov 15, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

myronmarston commented Nov 15, 2016

Uh oh!

JonRowe commented Nov 15, 2016

Uh oh!

myronmarston left a comment

Choose a reason for hiding this comment

Uh oh!

myronmarston Nov 15, 2016

Choose a reason for hiding this comment

Uh oh!

myronmarston Nov 15, 2016

Choose a reason for hiding this comment

Uh oh!

JonRowe commented Nov 15, 2016

Uh oh!

myronmarston commented Nov 15, 2016

Uh oh!

yujinakayama commented May 4, 2017

Uh oh!

Uh oh!

JonRowe commented Nov 15, 2016 •

edited

Loading