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

Keep your description short #4

Open
andreareginato opened this issue Oct 3, 2012 · 12 comments
Open

Keep your description short #4

andreareginato opened this issue Oct 3, 2012 · 12 comments
Labels

Comments

@andreareginato
Copy link
Collaborator

Write your thoughts about the "keep your description short" best practice.

@nirvdrum
Copy link

nirvdrum commented Oct 3, 2012

40 characters is totally arbitrary. A description should be as long as it needs to be to convey the information it needs to convey. Be concise, sure. But don't hold yourself to some magical limit, IMHO.

@iain
Copy link

iain commented Oct 5, 2012

Also, be careful with automatic spec descriptions. Consider this spec:

let(:user) { FactoryGirl.create(:user) }

its(:owner) { should eq user }

When you run this with the formatter doc, or when the spec fails, the spec name is completely unreadable.

@inancgumus
Copy link

I think the output formatter is somewhat verbose. Because, in the example it doesn't matter whether it responses with 422 or not. It should response correctly, that's all matters. Telling about the output values are verbose.

@ZenCocoon
Copy link

I believe the "good" example should use is_expected.to instead of should as mentioned in the section "Expect vs Should syntax"

@dankohn
Copy link
Collaborator

dankohn commented Jul 24, 2014

Yes, I fixed this with #115 and it was merged but @andreareginato has been slow pushing the new version to live.

@ZenCocoon
Copy link

Great news, thanks.

@mattvanhorn
Copy link

"not valid" conveys a lot less useful information than "has unexpected parameters"
I think it is bad practice to encourage vague specifications.
I shouldn't have to go searching for the subject or setup to find out what "not valid" means.
See also: http://xunitpatterns.com/Obscure%20Test.html#Mystery%20Guest

@jmonsanto
Copy link

IMHO, 40 characters is really limiting -- may even result to much more confusion for the sake of meeting the 40 char limit. Conciseness is key though.

It's better if rephrased like this:

A spec description ideally should never be longer than 40 characters. If this happens you should split it using a context.

@aesyondu
Copy link

aesyondu commented Apr 2, 2020

I've read the first three best practices now (describe, context, description length), and in 2/3, I agree to the counter arguments in the discussion:

#2 (comment)
#4 (comment)

Maybe I'm just missing something, it could possibly be because of my lack of experience.

@ChaelCodes
Copy link

I think the example context here is vague and unhelpful compared to the original description. I like the 40 characters guideline, but the example is weak.

context 'when not valid' do
  it { is_expected.to respond_with 422 }
end

I think it'd be better to recommend a clear context:

context 'with unexpected params' do
  it { is_expected.to respond_with 422 }
end

@akz92
Copy link
Member

akz92 commented Aug 19, 2020

I agree @ChaelCodes, your suggestion is much more descriptive of the actual context

@lortza
Copy link

lortza commented May 17, 2023

I also agree with @ChaelCodes because I prefer specificity and try to write code in a dont-make-future-me-have-to-think-too-hard-about-this kind of way.

Along those lines, I could argue that some well-named variables could reduce more cognitive overhead when reading the various expected values that don't have obvious meaning (such as http response status codes from these examples). This allows us to keep our descriptions concise while also making each it statement easier to instantly understand.

# I hoisted these because I'm pretending that there are more scenarios 
let(:unauthorized) { 401 }
let(:unprocessable_content) { 422 }

context 'when logged out' do
  it { is_expected.to respond_with unauthorized }
end

context 'with unexpected params' do
  it { is_expected.to respond_with unprocessable_content }
end

This is a small side-step from the conversation directly about context copy, but still covers ways in which organic documentation can make code easier to read -- and I haven't seen that elsewhere in the docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests