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

How to describe your methods #2

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

How to describe your methods #2

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

Comments

@andreareginato
Copy link
Collaborator

@andreareginato andreareginato commented Oct 3, 2012

Write your thoughts about the "how to describe your methods" best practice.

@farski
Copy link

@farski farski commented Oct 3, 2012

Why use (dot) classMethod rather than ::classMethod? It seems like in other places where the distinction is made with symbols (eg, ruby doc), it's :: and #

@tilsammans
Copy link

@tilsammans tilsammans commented Oct 3, 2012

For me this notation is too technical. RSpec was created to express specs/tests using human language.

@MattRogish
Copy link

@MattRogish MattRogish commented Oct 3, 2012

+1 to ::

@jherdman
Copy link

@jherdman jherdman commented Oct 3, 2012

@farski I think that's a matter of taste given that both :: and . are acceptable notation for class methods. It's probably more important that your team agree on one or the other :)

@tilsammans depending on the context of the examples, I agree. When dealing with small units (e.g. a model), I tend to focus on describing the behaviour of individual methods. When dealing with something like a Rails controller, I'll describe the behaviour of the request, in which case I'd do something like this:

describe "GET #new" do
  # ...
end

describe "POST #create" do
  context "an admin user" do
  end

  context "an authenticated user" do
  end
end
@Spaceghost
Copy link

@Spaceghost Spaceghost commented Oct 3, 2012

How I write specs

This is how I've been writing my specs lately. I'm intentionally deviating from the norm to explore and find out if I can find some better practices. One of my goals is to explore writing a formatter that outputs rdoc from passing tests.

require 'spec_helper'
require 'blocking_spec_helper'
require 'persistence/post'

describe 'app/persistence/post.rb' do

  before do
    @post = ::Persistence::Post.new title: Faker::Name.name, body: Faker::Lorem.paragraph, author: "Johnneylee"
  end

  describe ::Persistence::Post, 'validates' do
    %w[title body author].each do |attribute|
      specify "presence of #{attribute}" do
        setter = attribute + "=" 
        @post.send setter.to_sym, nil
        @post.should be_invalid      
      end
    end

    specify 'uniqueness of title' do
      title = Faker::Name.name
      ::Persistence::Post.create! title: title, body: Faker::Lorem.paragraph, author: Faker::Name.name
      @post.title = title
      @post.should be_invalid
    end
  end

  describe ::Persistence::Post, 'methods' do
    describe "#author" do
      it 'should be a string' do
        @post.author.should be_an_instance_of String
      end
    end
  end
end

Which outputs this.

app/persistence/post.rb
  Persistence::Post validates
    presence of title
    presence of body
    presence of author
    uniqueness of title
  Persistence::Post methods
    #author
      should be a string

Finished in 0.70489 seconds
5 examples, 0 failures

Closing

I want to achieve a level of bliss where I can write expressive specs that output real documentation that can be used in a release.

@binarycode
Copy link

@binarycode binarycode commented Oct 5, 2012

@Spaceghost the it 'uniqueness of title' do parts look quite ugly, i think it's much clearer to write specify 'uniqueness of title' do instead

@Spaceghost
Copy link

@Spaceghost Spaceghost commented Oct 5, 2012

@binarycode I definitely agree. I wrote a small method that just wraps #it and I use that. I called it #specify as well. But it seems it was already in rspec!

@dchelimsky
Copy link

@dchelimsky dchelimsky commented Oct 6, 2012

@Spaceghost would you mind editing your comment above to use "specify" instead of "it" in that case? It'll help me sleep at night. Same with it "presence of attribute". Thx.

@evanphx
Copy link

@evanphx evanphx commented Oct 6, 2012

If your describe is ONLY a method name, like describe ".admin?" then you're not longer describing behaviors, you're now writing unit tests.

The point, generally, is to describe functionalities in terms of observable behaviors and then the code shows how those behaviors are manifested. Here is an example:

describe "Admin user" do
  before :each do
    @user = AdminUser.new
  end

  it "should be detectable" do
    @user.admin?.should be_true
  end
@bobbytables
Copy link

@bobbytables bobbytables commented Oct 6, 2012

Just a general rule of thumb I have is to never use "should" in my example. Your spec is saying what it does, not what it should be doing.

it "validates presence of name"

@dchelimsky
Copy link

@dchelimsky dchelimsky commented Oct 6, 2012

@evanphx consider:

describe Stack do
  context "when empty" do
    it "returns nil for #peek"
  end
end

describe Stack do
  context "when empty" do
    describe "#peek" do
      it "returns nil"
    end
  end
end

describe Stack do
  describe "#peek" do
    it "returns nil when empty"
  end
end

Output:

Stack
  when empty
    returns nil for #peek

Stack
  when empty
    #peek
      returns nil

Stack
  #peek
    returns nil when empty

Each of these approaches has pros and cons and I can't really argue that one of them is always correct or incorrect, but I think the last one, which uses describe "#peek" is perfectly reasonable due to the context of describe Stack.

@Spaceghost
Copy link

@Spaceghost Spaceghost commented Oct 6, 2012

@dchelimsky The funny part was that I have this in my common_spec_helper.rb in a private repo for work.

def specify description, &blk
  self.send :it, description, blk
end

I never saw the #specify or #example methods before. Thanks for being a champ.

@Spaceghost
Copy link

@Spaceghost Spaceghost commented Oct 6, 2012

@evanphx While I agree that you should be describing the behaviours of your objects rather than the implementations in your describe blocks, I'm intentionally not doing that so I can explore outside of the recommended use cases.

@dchelimsky how do you feel about how I was writing my specs above? Too weird?

Maybe there's a good point in here. If I wrote proper behavioral specs and then unit tests separately, that might do me a bit of good in terms of separating out capturing the domain.

@dchelimsky
Copy link

@dchelimsky dchelimsky commented Oct 6, 2012

@Spaceghost I'd let "validates" show up at the front of the validation examples - I didn't see "validates" at first and didn't understand what specify "presence of title" meant. Also, the only case in that example that maybe shows what a valid post looks like is "uniqueness of title", because it happens to use create!, and I think that means it'll raise an error if it doesn't get valid data.

Here's what I'd write for the same set of examples:

require 'spec_helper'
require 'blocking_spec_helper'
require 'persistence/post'

describe ::Persistence::Post do
  def build_post(overrides={})
    ::Persistence::Post.new { title: Faker::Name.name, body: Faker::Lorem.paragraph, author: Faker::Name.name }.merge(overrides)
  end

  def create_post(overrides={})
    build_post(overrides).save!
  end

  it "is valid with valid attributes" do
    build_post.should be_valid
  end

  %w[title body author].each do |attribute|
    it "validates presence of #{attribute}" do
      build_post( attribute.to_sym => nil ).should_not be_valid
    end
  end

  it 'validates uniqueness of title' do
    pre_existing_post = create_post
    build_post(title: pre_existing_post.title).should_not be_valid
  end
end
@dchelimsky
Copy link

@dchelimsky dchelimsky commented Oct 6, 2012

@Spaceghost I accidentally hit submit and had to go back and edit that (a few times). Looks like what I'd do now.

@pepe
Copy link

@pepe pepe commented Oct 11, 2012

@dchelimsky thanks for array of attributes example, but should not there be one more end after dynamic example.

And for describing methods, I am on the @evanphx side. I am always trying to describe behavior. Except for things like it { should be_empty }.

@dchelimsky
Copy link

@dchelimsky dchelimsky commented Oct 12, 2012

@pepe - yep, fixed, thanks.

@dchelimsky
Copy link

@dchelimsky dchelimsky commented Oct 12, 2012

@pepe, @evanphx I want the descriptions to give me an overview of what the behavior is and the code examples to give me the detail, but "overview" means different things in different contexts. For example, I would say "Money supports addition using +" rather than just "Money supports addition" because I can read that in the output and learn quite a bit of useful information from that. I would be very unlikely, however, to cite a specific examplee.g. "Money returns $5 for $4 + $1".

That said, I think describing a specific return value is reasonable when talking about a behavior like "Array[index bigger than size] returns nil" it describes the Array's behavior in response to a class of inputs.

@andypalmer
Copy link

@andypalmer andypalmer commented Oct 18, 2012

I originally disagreed with this recommendation, then I read @dchelimsky's post and thought "Hmm, this works well for documenting APIs".
I'm not a big fan of RDOC, JavaDoc etc, and I'm still on the fence about this one. Perhaps it would be less controversial if the recommendation were titled "How to describe your public API"?

@Spaceghost
Copy link

@Spaceghost Spaceghost commented Dec 5, 2012

@dchelimsky I know you've handed the reins over to @alindeman and @myronmarston for rspec, but I really like your methods #build_post and #create_post.

I'm going to do some braining on this, partly with my fingers, and see what rises out of it.

Thank you again for all the love/hate/beer you've had in the name of RSpec.

@dchelimsky
Copy link

@dchelimsky dchelimsky commented Dec 5, 2012

@Spaceghost I actually picked that up from somebody else. When I remember who I'll try to also remember to post back here to give credit where credit is due.

@zolzaya
Copy link

@zolzaya zolzaya commented Jan 29, 2013

👍

tony612 added a commit to tony612/betterspecs that referenced this issue Mar 19, 2013
@brett-richardson
Copy link

@brett-richardson brett-richardson commented Dec 27, 2013

I just wrote a small gem that allows a much more terse syntax in this very situation.

https://github.com/brett-richardson/rspec-describe-method

The syntax is like this:

describe 'test' do
  describe_method '#upcase' do
    it{ should eq 'TEST' }
  end

  when_calling '#concat', 'ing' do # Alternative syntax (and arguments)
    it{ should eq 'testing' }
  end
end

Essentially you can infer the value of subject by using the following call: describe_method "#upcase" do
The gem detects instance and class methods and delegates them to the required object.

Essentially, under the hood... this is happening:
subject{ super().send method_name[1..-1], *args } (when it's an instance method).

Allows some really nice specs. Also allows nesting.

Does anyone see any value in this? Let me know.

mkaschenko added a commit to tdd-tales/Charles_Perrault-Puss_in_Boots that referenced this issue Jul 9, 2014
mkaschenko added a commit to tdd-tales/Charles_Perrault-Puss_in_Boots that referenced this issue Jul 9, 2014
@dideler
Copy link

@dideler dideler commented Oct 9, 2014

Something the guideline isn't clear on: What's the suggested way of describing a method that takes arguments? For example

def foo(bar)
  ...
end

describe "#foo(bar)"
# vs
describe "#foo"
@Spaceghost
Copy link

@Spaceghost Spaceghost commented Oct 9, 2014

@dideler When describing a class method, I use . and for an instance method, I use #.

I also include the method signature in the string like #foo(bar=:bar, baz={})

@dideler
Copy link

@dideler dideler commented Oct 10, 2014

Thanks @Spaceghost

@adibsaad
Copy link

@adibsaad adibsaad commented Sep 14, 2015

Fyi :: is called the scope-resolution operator.

@diegoguemes
Copy link

@diegoguemes diegoguemes commented Oct 22, 2015

As some people already said, I think this syntax is amazing for describing methods, but it's not good enough for describing behaviors. It won't be useful for developers that are not familiar with the code.

Maybe I'm getting too far here, but I see it like redundant comments for getter and setters that don't add any additional value.

describe '#purchase' do
   # ...
   item.purchase
   # ...

Doesn't seem very DRY.

@dideler
Copy link

@dideler dideler commented Oct 24, 2015

For reference, here's the guideline

BAD

describe 'the authenticate method for User' do
describe 'if the user is an admin' do

GOOD

describe '.authenticate' do
describe '#admin?' do

@diegoguemes

it's not good enough for describing behaviors

The behaviour is described by the inner blocks. E.g.

describe Item do
  describe '#purchase' do
    context 'when given a quantity' do
      it 'purchases n items'
    end
  end
end

I see it like redundant comments for getter and setters that don't add any additional value.

The naming convention provides consistency across tests and ensures related tests are grouped, so readers can easily navigate and find what they're looking for, and test output clearly shows what part of the system is under test.

Doesn't seem very DRY.

Be careful not to make your tests too DRY. Tests should be DAMP (Descriptive and Meaningful Phrases) which favours readability over anything else while still eliminating duplication where possible if it enhances readability.

@wakproductions
Copy link

@wakproductions wakproductions commented Oct 24, 2015

👍 @dideler right on

@jerzygangi
Copy link

@jerzygangi jerzygangi commented Mar 20, 2016

I fundamentally disagree with describing methods -- I write specs to describe behavior. For me, one of the most useful parts of RSpec is building a living documentation of what my application is doing -- in non-technical lingo. I intentionally do not refer to methods inside my it or describe or context descriptions. The result is a very easy to read, easy to scan, documentation of what my code is doing.

@jerzygangi
Copy link

@jerzygangi jerzygangi commented Mar 20, 2016

(And rspec --format documentation is great for this)

@mkaschenko
Copy link

@mkaschenko mkaschenko commented Mar 20, 2016

@jerzygangi could you provide an example?

@Spaceghost
Copy link

@Spaceghost Spaceghost commented Mar 21, 2016

One thing to consider is that there are different kinds of tests. Unit,
integration, and functional. I always describe behaviour, but at different
granularities depending on what kind of test it is.

There's definitely a lot of space for multiple distinct approaches
depending on the type of test you're writing. I write tests a bit more
similar to what I think you're describing, but those are functional tests
that express the inputs and outputs of the system, the parts the user
interacts with.

~Johnneylee

On Sat, Mar 19, 2016 at 9:19 PM, Maxim Kashchenko notifications@github.com
wrote:

@jerzygangi https://github.com/jerzygangi could you provide an example?


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#2 (comment)

@irisbune
Copy link

@irisbune irisbune commented Mar 22, 2016

@jerzygangi That is exactly what I find so confusing.

I'm learning RoR and RSpec now. In the bootcamp I joined, it was made clear that RSpec is meant to provide test output that is understandable to all, especially non-programmers. This GOOD vs BAD example does the exact opposite?

@spaghetticode
Copy link

@spaghetticode spaghetticode commented Jun 3, 2019

I think that, when describing class methods, the :: syntax should be preferable to .

My reasons for preferring :::

A `.' matches either class or instance methods, while #method matches only instance and ::method matches only class methods.

What do you think?

@shasticdev
Copy link

@shasticdev shasticdev commented Oct 10, 2019

I wonder what is the correct way to write the description of a scope.
should it be:
describe ".my_scope"
or
describe "#my_scope"

@abuisman
Copy link

@abuisman abuisman commented Feb 24, 2020

@shasticdev I do:

describe (scope.some_scope_name)
   it '...'
end

At least it communicates that it is a scope ;)

@abuisman
Copy link

@abuisman abuisman commented Feb 24, 2020

To join in on the discussion with :: vs . I prefer ., it is a less distracting notation and using . for calling class methods is something I see done a lot more. Also, it is similar to css's class selector, which is another anchor in many people's minds, so I think it is more natural.

@aesyondu
Copy link

@aesyondu aesyondu commented Apr 2, 2020

To join in on the discussion with :: vs . I prefer ., it is a less distracting notation and using . for calling class methods is something I see done a lot more. Also, it is similar to css's class selector, which is another anchor in many people's minds, so I think it is more natural.

I would argue that the fact that it's similar to css would confuse whoever reads the test. They might think you're testing html/css code. (Obviously this depends on who is reading your test cases, and I do think this is highly unlikely)

Also, . is so small it's easy to miss, unlike ::.

After reading this discussion spanning ~7 years, I can conclude that best practices are in the eyes of the beholder.

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

Successfully merging a pull request may close this issue.

None yet