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

Remove deprecations #2864

Merged
merged 10 commits into from
Feb 20, 2021
Merged

Remove deprecations #2864

merged 10 commits into from
Feb 20, 2021

Conversation

pirj
Copy link
Member

@pirj pirj commented Feb 13, 2021

This PR consists of a number of individual unrelated commits, better reviewed separately.

I don't mind sending them all as separate PRs. However, the changes are quite small and self-contained.

Depends on:

Not Done

There are a few things I'm afraid/reluctant of touching, specifically:

@pirj
Copy link
Member Author

pirj commented Feb 14, 2021

Green.

lib/rspec/core/formatters.rb Show resolved Hide resolved
spec/rspec/core/drb_spec.rb Show resolved Hide resolved
features/formatters/custom_formatter.feature Outdated Show resolved Hide resolved
lib/rspec/core.rb Show resolved Hide resolved
#
# it "does something", :pending => "something else getting finished" do
# # ...
# end
Copy link
Member

Choose a reason for hiding this comment

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

This could be reworded rather than removed, perhaps mentioning that only the failures in the block cause the behaviour?

Copy link
Member Author

Choose a reason for hiding this comment

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

The reason I've removed this was that If you want to declare an example pendingand bypass thebefore hooks as well was misleading. Replaced it with pending example method docs.

Copy link
Member

Choose a reason for hiding this comment

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

Actually this note needs to be here, as this is the doc for the pending example method? It is relevant to explain that before hooks are not skipped, so the alternative is the pending metadata.

Copy link
Member Author

Choose a reason for hiding this comment

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

Sorry, I don't understand what needs to remain.
As far as I understand, hooks aren't skipped in either case. In this regards, there is no alternative.
And all of:

pending 'example method' do
  # something that fails
end
it 'has a cool feature', pending: 'yet to be implemented' do # or just pending: true
  # cool missing feature test
end
it 'should work' do
  pending 'but does not'
  # and a failing expectation
end

are just different notations of the same example group that will pass if its expectations fail.

Am I missing something? I'm all for clarifying/extending the docs, but at this point I'm out of ideas on how to do this exactly.

Copy link
Member

Choose a reason for hiding this comment

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

This doc is explaining this difference:

RSpec.describe do
  before do
    raise "BOOM!"
  end

  it "will pend", pending: "Oh yes" do
    raise "Yeah nah"
  end

  it "will fail" do
    pending do
      raise "Won't happen"
    end
  end
end
*F

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

  1) will pend
     # Oh yes
     Failure/Error: raise "BOOM!"

     RuntimeError:
       BOOM!
     # ./spec/pending_example_spec.rb:3:in `block (2 levels) in <top (required)>'

Failures:

  1) will fail
     Failure/Error: raise "BOOM!"

     RuntimeError:
       BOOM!
     # ./spec/pending_example_spec.rb:3:in `block (2 levels) in <top (required)>'

Finished in 0.00349 seconds (files took 0.13413 seconds to load)
2 examples, 1 failure, 1 pending

Failed examples:

rspec ./spec/pending_example_spec.rb:10 # will fail


Copy link
Member

Choose a reason for hiding this comment

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

The difference being that pending metadata will cause a hook failure to be considered as part of the pending nature of the spec, where as the block will cause a failure as it hasn't reached the bit that can be pending.

Copy link
Member Author

Choose a reason for hiding this comment

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

Got it, thanks for the clarification!
Let me quickly get this note back.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.
I've extended the previous docs based on the assumption that if it wasn't clear at all for me, it wasn't clear for the majority.

I can extract those docs changes into separate PR, one for 4-0-dev, and another for main if you like.
Surely it doesn't relate to "Remove deprecations" much, especially in the part of pending.rb where only the error message was reduced not to mention RSpec 2.

Copy link
Member Author

Choose a reason for hiding this comment

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

lib/rspec/core/pending.rb Show resolved Hide resolved
@pirj pirj force-pushed the remove-deprecations-2 branch 2 times, most recently from 8d420b2 to 8470322 Compare February 15, 2021 16:44
Comment on lines 56 to 73
# method within an example. If you want to declare an example `pending`
# and bypass the `before` hooks as well, you can pass `:pending => true`
# to the `it` method:
# @example Alternatively, you can use the `pending` example method:
#
# it "does something", :pending => true do
# describe "an example" do
# pending "does not implement something yet" do
# # ...
# end
# end
#
# or pass `:pending => "something else getting finished"` to add a
# message to the summary report:
# or specify metadata on an example:
#
# it "does something", :pending => "something else getting finished" do
# # ...
# end
# it "does this", :pending => "is not yet implemented" do
# # ...
# end
#
# even without an explicit pending message:
#
# it "does something", :pending do
# # ...
# end
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure if it's the best place for this doc.
Putting it to

define_example_method :pending, :pending => true
would be better, but I'm not sure how to refer from this doc to example_group.rb, would @see {ExampleGroup#pending} work? https://rubydoc.info/gems/yard/file/docs/GettingStarted.md#linking-objects

Copy link
Member

Choose a reason for hiding this comment

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

This whole doc is about pending examples, if you'd like to change it can we do so in another PR with a diff of the rendered docs?

Copy link
Member Author

Choose a reason for hiding this comment

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

A summary of what is done here:

  1. Replaced a confusing "an example" docstring of a describe statement:
- describe "an example" do
+ describe "some behaviour" do
  1. Removed the misleading notion of a possibility to bypass hooks for any notation of a pending example:
- #`before(:example)` hooks are eval'd when you use the `pending`
- #   method within an example. If you want to declare an example `pending`
- #   and bypass the `before` hooks as well,

just because hooks are run either way.

  1. Wrapped the example of pending example group alias into describe to make a distinction between pending example alias and pending example-scope method:
it "is pending with no message" do
  pending
end

vs

describe "an example" do
  pending "does not implement something yet" do
    # ...
  end
end

I admit I've repeated the mistake with "an example" docstring I've fixed in 1) 😆
Let me fix this real quick.

There is no massive change here, but surely it's hard to grasp what was made just by looking at the diff.

@pirj pirj requested a review from JonRowe February 18, 2021 11:06
spec/rspec/core/runner_spec.rb Outdated Show resolved Hide resolved
#
# it "does something", :pending => "something else getting finished" do
# # ...
# end
Copy link
Member

Choose a reason for hiding this comment

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

Actually this note needs to be here, as this is the doc for the pending example method? It is relevant to explain that before hooks are not skipped, so the alternative is the pending metadata.

@pirj pirj force-pushed the remove-deprecations-2 branch 2 times, most recently from 1ee0f91 to c7d5bc6 Compare February 18, 2021 22:25
This was referenced Feb 20, 2021
We used to mention RSpec 2 there which is long gone.

Also, this note was misleading:

> If you want to declare an example `pending` and bypass the `before`
hooks as well, you can pass `:pending => true`

In the following spec:

    RSpec.describe do
      before { puts 'before!' }

      it do
        pending 'oops'
        fail 'djosdkfjos'
      end

      it 'pending metadata', pending: true do
        fail 'dferrerere'
      end
    end

the hook is run for both metadata and in-example `pending`:

    before!
      pending metadata (PENDING: No reason given)
    before!
      example at ./spec/a_spec.rb:4 (PENDING: oops)
@pirj pirj merged commit e0f638d into 4-0-dev Feb 20, 2021
@pirj pirj deleted the remove-deprecations-2 branch February 20, 2021 10:02
pirj added a commit to rspec/rspec-rails that referenced this pull request Sep 18, 2021
The option was removed in RSpec 4, see rspec/rspec-core#2864
Follow-up to #2458
pirj added a commit to rspec/rspec-rails that referenced this pull request Sep 18, 2021
The option was removed in RSpec 4, see rspec/rspec-core#2864
Follow-up to #2458
yujinakayama pushed a commit to yujinakayama/rspec-monorepo that referenced this pull request Oct 19, 2021
…recations-2

Remove deprecations

---
This commit was imported from rspec/rspec-core@e0f638d.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants