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

RSpec/ContextWording with nested contexts #510

Open
jcoyne opened this issue Dec 8, 2017 · 15 comments
Open

RSpec/ContextWording with nested contexts #510

jcoyne opened this issue Dec 8, 2017 · 15 comments

Comments

@jcoyne
Copy link

jcoyne commented Dec 8, 2017

How should one use the RSpec/ContextWording with nested contexts? Should it support 'and with/when' such that when I do:

describe 'The program' do
  context 'when configured' do
    context 'and when the user is valid' do
      it 'does the right thing'
    end
  end
end

and run $ rspec spec --format documentation

it shows a readable sentence?
"The program when configured and when the user is valid does the right thing"

Presently my example triggers:
Start context description with 'when', 'with' or 'without'.

If I follow that advice, the documentation is less readable:
"The program when configured when the user is valid does the right thing"

@bquorning
Copy link
Collaborator

cc @pirj

@Darhazer
Copy link
Member

when configured with valid user will pass the cop and IMO is better description.

@jcoyne
Copy link
Author

jcoyne commented Dec 11, 2017

@Darhazer I think you are implying that it is configured with the user, but I was saying something more along the lines of: when the program has a correct configuration and a valid user

@pirj
Copy link
Member

pirj commented Dec 11, 2017

It makes sense to open this issue on https://github.com/reachlocal/rspec-style-guide or https://github.com/lelylan/betterspecs, we're just soldiers here, we don't create rules.

@jcoyne
Copy link
Author

jcoyne commented Dec 18, 2017

@pirj I can appreciate that:

rubocop/rspec-style-guide#17

There's already a question about the and coupler on betterspecs/betterspecs#3 (comment)

@jcoyne
Copy link
Author

jcoyne commented Dec 19, 2017

@pirj I just talked with @bikeonastick (from ReachLocal) tonight and he said that they are no longer maintaining https://github.com/reachlocal/rspec-style-guide

@IlkhamGaysin
Copy link

@Darhazer it's ok for the example above but what if we would have more than one nesting?

feature "As a visitor, who has given a purchase link, I should not be allowed to see purchase page" do
  context "when the visitor has an email set up in session" do
    context "and visitor has corresponding user in db" do
      context "and visitor's domain of email is white-listed" do
      end
    end
  end
end

@dgollahon
Copy link
Contributor

I don't yet have a strong opinion on this, but one thought is that while "and" makes the final docstring seem neat and readable in some cases, it makes reading the spec itself feel a bit more awkward to me. "Here's the context: and {x}" feels strange to me in a way that "Here's the context: when {x}" doesn't.

@IlkhamGaysin I think your example is reasonable, but I'd be more likely to write it as:

feature "As a visitor, who has given a purchase link, I should not be allowed to see purchase page" do
  context "when the visitor has an email set up in session" do
    context "with a corresponding user in the db" do
      context "with a white-listed email domain" do
      end
    end
  end
end

But I'm not convinced one is clearly better than the other. I generally try to avoid having deeply nested contexts when i can so I don't think about this problem that often. I do sometimes end up with something like "with a corresponding user in the db and the user has a white-listed email domain" and just skipping a context level. That may be cheating for the purpose of the discussion though.

One other very minor thought is that if you allow "and" it wouldn't be flagged if there was no parent context string for it to be connected onto, but I confess that I expect that case is exceedingly rare.

Individual users, of course, are free to add 'and' to their rubocop config file as it suits them. I don't have a strong opinion about adding it or not adding it to the defaults at the moment though.

@pirj
Copy link
Member

pirj commented May 23, 2018

I was actually thinking of adding a when/with and without helpers, e.g.:

describe '...' do
  when 'the visitor has an email' do
    with 'a corresponding user' do
      without 'a confirmation' do
        ...
      end
    end
  end
end

Problem with an and is that it's a Ruby keyword ;)

@IlkhamGaysin
Copy link

@pirj looks good! 👍

@pirj
Copy link
Member

pirj commented Jul 24, 2019

Related to #757.
Context Descriptions guideline is now more permissive:

When an example block description is composed with context block descriptions, it should result in a sentence with proper grammar. This typically means the context should start with a term such as 'when', 'with', or 'without'.

Would you agree to configure prefixes for the cop on a per-project basis, or is it a major effort you wouldn't want to deal with, @jcoyne ?

There's a lengthy list of the frequently used prefixes I've gathered from real-world-rails.

@jcoyne
Copy link
Author

jcoyne commented Jul 29, 2019

I think the advice that "it should result in a sentence with proper grammar" leads one to the use of "and".

"when x is true when y is true" would be properly written "when x is true and when y is true"

Personally, I don't mind configuring my own prefixes, but by default you can't make proper sentences for the documentation formatter without the use of "and"

@bj-mcduck
Copy link

bj-mcduck commented Jun 4, 2020

@jcoyne You can just write a different sentence that reads better.
The program when configured with a valid user does the right thing

@IlkhamGaysin

feature "As a visitor, who has given a purchase link, I should not be allowed to see purchase page" do
  context "when the visitor has an email set up in session" do
    context "with a corresponding user in db" do
      context "with a white-listed email" do
      end
    end
  end
end

Granted in this context it makes it a little more reasonable that you might use and in a nested context, but I personally wonder if that starts being a code-smell when it gets too nested.
Maybe? Maybe not?

There is the option of turning off the cop, or perhaps it could be configurable to white-list other words in the cop?

I personally wouldn't want the and word added to start contexts with as the only places I've seen my team use it has been when their sentences are written confusingly anyway.

@pirj
Copy link
Member

pirj commented Jun 5, 2020

Do you think there is something actionable here, or can this be closed?

@jcoyne
Copy link
Author

jcoyne commented Jun 5, 2020

I think "and when" and "and with" could be added to the default acceptable prefixes.

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

No branches or pull requests

7 participants