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

Update Rspec/ContextWording list #186

Closed
wants to merge 1 commit into from
Closed

Update Rspec/ContextWording list #186

wants to merge 1 commit into from

Conversation

colinbruce
Copy link

We are implementing the rubocop-govuk gem in a couple of repos and are having some linguistic issues with tests that exercise delayed jobs.

We have a number of tests where the nested contexts are:

context 'when submission errors' do
  context 'with a generic error' do
   	# setup
    context 'before the final retry' do
      # setup
      # test
    end
    context 'when the retry is at the max retries' do
      # setup
      # test
    end
  end
end

Rewording the tests in line with the required words (and the final retry has not started) makes them harder to understand!
As the rubocop documenttation also lists before and after as acceptable prefixes, how do you feel about this extension?

We are implementing the rubocop-govuk gem in a couple of repos and
are having some linguistic issues with tests that exercise delayed
jobs.  We have a number of tests where the nested contexts are:
```ruby
context 'when submission errors' do

  context 'with a generic error' do
   	# setup

    context 'before the final retry' do
      # setup
      # test
    end

    context 'when the retry is at the max retries' do
      # setup
      # test
  	end
  end
end
```

and rewording the tests inline with the required words (`and the final retry has not started`)
make them harder to understand!
As the [rubocop documenttation](https://www.rubydoc.info/gems/rubocop-rspec/RuboCop/Cop/RSpec/ContextWording) also lists `before` and `after` as acceptable prefixes, how do you feel
about this extension?
@kevindew
Copy link
Member

Hey @colinbruce thanks for opening a PR here, I appreciate the contribution. Unfortunately this isn't something we'd like to accept.

The reasons for this are:

The course of action I'd suggest is to refactor your tests to cut out the nested contexts - I know, based on our past, it's easier said than done if you've got heavy use of them - but, also based on our ex, they can be really complex to follow.

I.e for the example you've given I'd prefer to read code like:

context 'when a submission error occurs before the final retry' do
  # setup
  # test
end

context 'when an error occurs and there are no more retries' do
  # setup
  # test
end

Hope this helps

@colinbruce
Copy link
Author

Thanks for responding @kevindew

@colinbruce colinbruce closed this Feb 22, 2022
@colinbruce colinbruce deleted the update-rspec-context-wording branch February 22, 2022 07:25
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.

None yet

2 participants