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

v1.43.0 Regression: EmptyExampleGroup incorrectly flags custom example groups w/ block arguments #1006

Closed
jtannas opened this issue Aug 19, 2020 · 8 comments

Comments

@jtannas
Copy link
Contributor

jtannas commented Aug 19, 2020

Running v1.43.1, I've run into a regression where the EmptyExampleGroup cop is now incorrectly flagging custom includes that take a block argument. In my case, it's a run_test! method that makes a request and expects the response to match a swagger spec, and optionally takes additional expectations within a block.

Reproducible with the following failing test:

  context 'when a custom include method takes a block argument' do
    let(:cop_config) do
      { 'CustomIncludeMethods' => ['run_test!'] }
    end

    it 'ignores an empty example group with a custom include and a block' do
      expect_no_offenses(<<~RUBY)
        describe Foo do
          context "when I do something clever" do
            run_test! do
              expect(foo).to be(true)
            end
          end
        end
      RUBY
    end
  end

Failure:

Failures:

  1) RuboCop::Cop::RSpec::EmptyExampleGroup when a custom include method takes can a block argument ignores an empty example group with a custom include and a block
     Failure/Error: super
     
       expected: "describe Foo do\n  context \"when I do something clever\" do\n    run_test! do\n      expect(foo).to be(true)\n    end\n  end\nend\n"
            got: "describe Foo do\n  context \"when I do something clever\" do\n  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^...pty example group detected.\n    run_test! do\n      expect(foo).to be(true)\n    end\n  end\nend\n"
     
       (compared using ==)
     
       Diff:
       @@ -1,5 +1,6 @@
        describe Foo do
          context "when I do something clever" do
       +  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Empty example group detected.
            run_test! do
              expect(foo).to be(true)
            end
       
     # ./spec/support/expect_offense.rb:17:in `expect_no_offenses'
     # ./spec/rubocop/cop/rspec/empty_example_group_spec.rb:236:in `block (3 levels) in <top (required)>'
@pirj
Copy link
Member

pirj commented Aug 19, 2020

I guess run_test! is an alias or a wrapper around it, example description method?

This is going to be resolved with #956 with configurable RSpec DSL. I.e. you'll be able to extend your configuration:

AllCops:
  RSpec:
    Language:
      Examples:
        Regular:
        - 'run_test!'

@jtannas
Copy link
Contributor Author

jtannas commented Aug 19, 2020

I guess run_test! is an alias or a wrapper around it, example description method?

yup, it's a wrapper that essentially does:

it 'matches the expected format' do
	submit_request
	assert_response_matches_metadata # http code, headers, json structure, etc...
	yield
end

See here for details: https://github.com/rswag/rswag#paths-operations-and-responses

@pirj
Copy link
Member

pirj commented Aug 19, 2020

Got it. I recommend disabling the cop temporarily and turning it back on when #956 is merged and released. It's going to happen in 2.0, and I'd love to say that it will happen really soon, but we depend on the parent project with the major release.

@single-stop-tony
Copy link

@pirj I'm assuming this won't fix a similar issue that I'm having. We have some shared examples in our project with one-liner tests inside of conditionals/ternaries. Those shared examples are now being flagged as empty due to this change. I understand that examples inside of conditionals might be violating some other principle, but it seems like a misnomer to be considered an "empty example group". Should I open a separate issue?

@pirj
Copy link
Member

pirj commented Aug 21, 2020

@single-stop-tony Quite an interesting case.

Have you considered using RSpec's conditional filters?

Yes, please open a separate ticket.

@single-stop-tony
Copy link

I have not! Might be a useful cop to add as well

@pirj
Copy link
Member

pirj commented Aug 21, 2020

Mercy! Our backlog of useful cops is overflown :D

@pirj
Copy link
Member

pirj commented Nov 6, 2020

Duplicated by #1050

#956 has been released as part of 2.0, CustomIncludeMethods have been removed in #1007 that is also part of 2.0.

Please let us know if RSpec/EmptyExampleGroup works properly now, and feel free to reopen if it doesn't.

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 a pull request may close this issue.

3 participants