Skip to content
This repository has been archived by the owner on Nov 30, 2024. It is now read-only.

Skip backtrace for pending examples #2957

Merged
merged 1 commit into from
Jan 7, 2023
Merged

Conversation

pirj
Copy link
Member

@pirj pirj commented Apr 24, 2022

fixes #2956

# spec/spec_helper.rb
RSpec.configure do |c|
  c.pending_failure_output = :no_backtrace
  # ...
end

🎉

$ rspec spec/a_spec.rb
Run options: exclude {:ruby=>#<Proc: ./spec/spec_helper.rb:111>}

Randomized with seed 9609


  pending (PENDING: No reason given)
  fails (FAILED - 1)

Pending: (Failures listed here are expected and do not affect your suite's status)

  1) pending
     # No reason given
     Failure/Error: expect(1).to eq(2)

       expected: 2
            got: 1

       (compared using ==)

Failures:

  1) fails
     Failure/Error: expect(1).to eq(2)

       expected: 2
            got: 1

       (compared using ==)
     # /Users/pirj/source/rspec-dev/repos/rspec-support/lib/rspec/support.rb:102:in `block in <module:Support>'
     # /Users/pirj/source/rspec-dev/repos/rspec-support/lib/rspec/support.rb:111:in `notify_failure'

@pirj pirj self-assigned this Apr 24, 2022
@pirj pirj marked this pull request as draft April 24, 2022 10:45
@pirj pirj requested a review from JonRowe April 24, 2022 10:45
@pirj
Copy link
Member Author

pirj commented Apr 24, 2022

Is this a correct approach?
I'll write specs for this and will handle making it the default in 4-0-dev.

@eregon
Copy link
Contributor

eregon commented Apr 24, 2022

I wonder if it should even be more succinct, because 99% (I guess) of the times the details about pending specs don't matter.
It could be shown maybe like the summary of failed examples:
Taking output of https://github.com/rspec/rspec-mocks/runs/6129989849?check_suite_focus=true:

Pending examples:

rspec ./spec/rspec/mocks/verifying_doubles/class_double_with_class_loaded_spec.rb:167 # A class double with the doubled class loaded when `.new` is stubbed on a class with a private `new` uses the method signature from `#initialize` for arg verification
rspec ./spec/rspec/mocks/partial_double_spec.rb:611 # when verify_partial_doubles configuration option is set when `.new` is stubbed on a class with a private `new` uses the method signature from `#initialize` for arg verification

Or maybe like the profiler output:

Pending examples:
  RSpec::Mocks behaves like library wide checks has no malformed whitespace
    /home/runner/work/rspec-mocks/rspec-support/lib/rspec/support/spec/library_wide_checks.rb:142
  Loading rspec/mocks/standalone exposes the RSpec::Mocks API on `main`
    ./spec/rspec/mocks/standalone_spec.rb:7
  #any_instance setting a message expectation with an expectation is set on a method that exists fails if an instance is created but no invocation occurs
    ./spec/rspec/mocks/any_instance_spec.rb:730

Both are visually distinctive from failures, but just removing the backtrace like above does not make it sufficiently different for me to realize it's not a failure.

@JonRowe
Copy link
Member

JonRowe commented Apr 24, 2022

I like this output, we don't minimise failures to single lines in the default formatters and we shouldn't here either but we could go further and make this setting something like: pending_failure_output = :full | :no_backtrace | :skip where we print as is (:full, the default for 3.x), print like this (:no_backtrace default for 4.x), or skip entirely that section (:skip, possibly what we'd use on our CI)

@pirj
Copy link
Member Author

pirj commented Apr 24, 2022

We don't seem to refer to pending as failures. What do you think of calling this setting just pending_output to avoid confusion?

PS Except in "Failures listed here are expected".
Still, a pending failure may be confused with a pending example that succeeded:

the rest of the example is still run but is expected to fail, and will be marked as a failure (rather than as pending) if the example passes.

@JonRowe
Copy link
Member

JonRowe commented Apr 24, 2022

Theres a bit of mixed terminiology within our code base, as prior to 3.x we only had pending to mean "some spec yet to pass, maybe, so skip" but 3.x split this into "Pending specs are expected failures, they are verified to fail." and "skipped specs are skipped" as far as I know skipped specs already don't raise a failure like case?

@pirj
Copy link
Member Author

pirj commented Apr 25, 2022

We seem to mark skipped examples as pending, too. And they appear as such contributing to the number of pending examples in the footer:

describe ".configuration", :skip do
25 examples, 0 failures, 1 pending

But yes, they are not run and won't fail the build (if they would pass).

@pirj pirj force-pushed the skip-backtrace-for-pending-examples branch from 8510ae1 to ab30c0c Compare May 2, 2022 18:23
@JonRowe
Copy link
Member

JonRowe commented May 2, 2022

This is looking good but I'd love a cucumber feature

@pirj
Copy link
Member Author

pirj commented May 2, 2022

My thought exactly. Cracked my teeth with trying to cover this change with specs.

@pirj pirj force-pushed the skip-backtrace-for-pending-examples branch from 1818d9d to c669cbc Compare May 3, 2022 16:11
@pirj pirj marked this pull request as ready for review May 3, 2022 16:11
@pirj
Copy link
Member Author

pirj commented May 3, 2022

We have a .nav file. And the clear_examples.feature is not listed there, and it's not in the generated docs. But even though it limits features to be shown under Configuration, all of them appear in the docs. So I guess it doesn't matter if we add it to .nav or not - it will appear in generated docs.

Copy link
Member

@JonRowe JonRowe left a comment

Choose a reason for hiding this comment

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

@pirj Can you rebase this? If it goes green as is when rebased please merge and add a changelog

@pirj pirj force-pushed the skip-backtrace-for-pending-examples branch from c669cbc to 9186c76 Compare January 7, 2023 15:31
@pirj pirj merged commit a27c169 into main Jan 7, 2023
@pirj pirj deleted the skip-backtrace-for-pending-examples branch January 7, 2023 16:22
@JonRowe
Copy link
Member

JonRowe commented Feb 4, 2024

Released in 3.13.0, apologies it has taken so long.

ivoanjo added a commit to DataDog/dd-trace-rb that referenced this pull request Nov 7, 2024
p-datadog pushed a commit to DataDog/dd-trace-rb that referenced this pull request Nov 7, 2024
* master: (53 commits)
  [NO-TICKET] Re-enable ASAN memory leaks testing
  [PROF-9470] Enable "heap clean after GC" profiler optimization by default
  [🤖] Lock Dependency: https://github.com/DataDog/dd-trace-rb/actions/runs/11724221529
  Force rspec >= 3.13 to pick up rspec/rspec-core#2957
  Minor style fix
  Add missing assertions/stubs to avoid noise in test output
  Hide skipped/pending tests from rspec output by default
  Revert "Disable crashtracking by default"
  Lockfile updates for libdatadog upgrade
  [NO-TICKET] Upgrade to libdatadog 14.1
  remove unused rspec integration app
  Update lockfiles for release 2.6.0
  Bump version 2.5.0 to 2.6.0
  Add 2.6.0 to CHANGELOG.md
  Remove unintentional libddwaf require
  Specify gem version for macos test
  Address PR comments
  Address new Ruby 3.4 error message changes
  Add macos and yjit tests
  [NO-TICKET] Relax profiling CODEOWNERS configuration
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Less verbose/clearer output for pending spec
3 participants