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

Wrong coverage models result with Shoulda-Matchers #638

Closed
SofiaSousa opened this issue Oct 29, 2017 · 16 comments
Closed

Wrong coverage models result with Shoulda-Matchers #638

SofiaSousa opened this issue Oct 29, 2017 · 16 comments

Comments

@SofiaSousa
Copy link

SofiaSousa commented Oct 29, 2017

Hi

I was trying to setup a rails api with RSpec, SimpleCov and Should-Matchers and I figured out that after running tests all models appears always 100% covered regardless of written tests, including models without tests.

If I comment the Should-Matchers setup, SimpleCov works correctly for models.

I added SimpleCov call to top of rails_spec.rb:

require 'simplecov'
SimpleCov.start 'rails'

And Shoulda-Matchers config to support/shoulda_matchers.rb

Shoulda::Matchers.configure do |config|
  config.integrate do |with|
    with.test_framework :rspec
    with.library :rails
  end
end

Anyone with the same problem?

Thanks!

@PragTob
Copy link
Collaborator

PragTob commented Oct 29, 2017

Ugh that doesn't sound good :'( Thanks for the report!

Thanks for the report. Do you happen to have a small sample application to demonstrate the problem?

@SofiaSousa
Copy link
Author

Yes, I have this sample https://github.com/SofiaSousa/pets_api

@erijonhson
Copy link

I'm having the same problem and thought I was covering everything :/

@nbekirov
Copy link

nbekirov commented Feb 12, 2018

@SofiaSousa I checked with the repo you mentioned and had no issues. Tested with both master and develop branches.
Coverage report generated for RSpec to /Users/nikolay/Projects/pets_api/coverage. 56 / 70 LOC (80.0%) covered.

Can share your coverage results and mark a few specific lines you think should not be marked as covered?

@SofiaSousa
Copy link
Author

A very simple example:

  • Remove or comment all model tests
  • Run Rspec and check models coverage

Result: Models (100.0% covered at 1.0 hits/line)

@nbekirov
Copy link

Remove or comment all model tests

Your classes are still required by the acceptance tests:

  • when using FactoryBot to create Pets and Species
  • when hitting the controllers

Looking into your models - they are pretty simple (i.e has no method definitions, conditions, etc.), and all lines are actually executed when required.

In the following example you can see two green lines:

  1. when Ruby executes the class keyword, it creates a new lexical scope for the new Pet class
  2. then belongs_to is invoked with the :specie argument

screen shot 2018-02-13 at 21 14 37

Code within module/classes is executed just as any other. You can test this by adding a simple puts to Pet

class Pet < ApplicationRecord
  puts 'Pet is now defined!'
  
  # more code
end

Then when you run rake the output is:

$ rake
/Users/nikolay/.rvm/rubies/ruby-2.4.0/bin/ruby -I/Users/nikolay/.rvm/gems/ruby-2.4.0/gems/rspec-core-3.7.0/lib:/Users/nikolay/.rvm/gems/ruby-2.4.0/gems/rspec-support-3.7.0/lib /Users/nikolay/.rvm/gems/ruby-2.4.0/gems/rspec-core-3.7.0/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb
Pet is now defined!

Pets
  GET /pets
    Listing pets
  GET /pets/:id
    Get a pet
  POST /pets
    Create a pet

Here's what happens if we define a new method within Pet:

  1. def greet is executed and and the new method is defined (it's added to the Pet method list; line is green)
  2. the actual method body is not invoked (line is red)

screen shot 2018-02-13 at 21 22 28

Please let me know what do you think of this. Maybe I'm missing something?

@PragTob
Copy link
Collaborator

PragTob commented Feb 14, 2018

@nbekirov thanks for all the help figuring this one out :)

@SofiaSousa
Copy link
Author

SofiaSousa commented Feb 14, 2018

@nbekirov, thanks for your reply. I totally agree with what you described, of course it makes sense :)

I prepared a branch with what I'm trying to explain: https://github.com/SofiaSousa/pets_api/tree/testing

No acceptance tests, no factories, no Shoulda-Matchers config. Only 'empty' model specs.

My goal was to check if models attributes validation is fully covered by my tests.

image

Correct me if I'm thinking wrong: how 12.5% of the code could be covered if there aren't any tests?

Yep, I don't have any method in the models yet. Does make sense check if validations are totally covered by tests?

@nbekirov
Copy link

nbekirov commented Feb 14, 2018

You haven't commented all of your specs code - for models Pet and Specie.

Commenting the lines gets me to:

Coverage report generated for RSpec to /Users/nikolay/Projects/pets_api/coverage. 0 / 91 LOC (0.0%) covered.

This line triggers the framework to load app/models/pet.rb:

RSpec.describe Pet, type: :model do

...then we proceed as described in my previous comment.

You can add this code to the Pet class and see for yourself:

class Pet < ApplicationRecord
  puts '-' * 100
  puts caller.grep(/pets_api/)
  puts '-' * 100

  # more code here
end

and run rake:

$ rake
/Users/nikolay/.rvm/rubies/ruby-2.4.0/bin/ruby -I/Users/nikolay/.rvm/gems/ruby-2.4.0/gems/rspec-core-3.7.0/lib:/Users/nikolay/.rvm/gems/ruby-2.4.0/gems/rspec-support-3.7.0/lib /Users/nikolay/.rvm/gems/ruby-2.4.0/gems/rspec-core-3.7.0/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb
----------------------------------------------------------------------------------------------------
/Users/nikolay/Projects/pets_api/app/models/pet.rb:1:in `<top (required)>'
/Users/nikolay/Projects/pets_api/spec/models/pet_spec.rb:3:in `<top (required)>'
----------------------------------------------------------------------------------------------------
No examples found.

Finished in 0.00031 seconds (files took 1.37 seconds to load)
0 examples, 0 failures

Coverage report generated for RSpec to /Users/nikolay/Projects/pets_api/coverage. 14 / 91 LOC (15.38%) covered.

... see how it was pet_spec.rb:3 that "required" our code.

Hope that helps! 😃

@SofiaSousa
Copy link
Author

I left those lines so coverage can find some test code and run. I understand the code in the model is executed but can we say that is tested?

@nbekirov
Copy link

Define "is tested" 😸

  • You tested that you have a Pet class (try having RSpec.describe Dog before writing the class itself)
  • You tested that the configuration is at least executable (make a typo in belong_to :specie and run rspec:
NoMethodError:
  undefined method `belong_to' for #<Class:0x007fafe0d7a838>
  Did you mean?  belongs_to
  • etc.

Code coverage says relatively little about the quality of your tests. For me "100% covered" is just a good starting point, it's in no way the end of the journey.

@SofiaSousa
Copy link
Author

SofiaSousa commented Feb 14, 2018

I was expecting to have to write tests for Pet name for example, so validates_presence_of :name could be assigned as covered. But maybe this is the job of the Rails core tests :)

@bf4
Copy link
Collaborator

bf4 commented Feb 14, 2018

Ruby's coverage module and SimpleCov which uses it only track C0 coverage. Any line that is evaluated when a file is loaded is considered 'covered', whether you not you executed it. #340

so

def foo
  puts "I am not covered unless you call 'foo'"
end

but

def foo; puts "I am covered when the file is required whether or not you call 'foo'"; end

@nbekirov
Copy link

I struggle to find online a document that describe C0 in a satisfying (suitable for different audiences) way that can be linked in a FAQ.

Here's the most useful ones I came across:

@PragTob
Copy link
Collaborator

PragTob commented Dec 3, 2019

I believe this mostly turned out to be a difference on what covered means for ruby etc. so I think there's nothing to do here and will hence close it. If you disagree/I missed something please let me know :)

Thanks everyone for the help figuring this out! 🎉

WhatsApp Image 2019-02-15 at 13 27 55

@PragTob PragTob closed this as completed Dec 3, 2019
@anton-kachan-dev
Copy link

Define "is tested" 😸

  • You tested that you have a Pet class (try having RSpec.describe Dog before writing the class itself)
  • You tested that the configuration is at least executable (make a typo in belong_to :specie and run rspec:
NoMethodError:
  undefined method `belong_to' for #<Class:0x007fafe0d7a838>
  Did you mean?  belongs_to
  • etc.

Code coverage says relatively little about the quality of your tests. For me "100% covered" is just a good starting point, it's in no way the end of the journey.

But if we write like this: belongs_to :whatever Tests will be passed. And I think it is not good.

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

6 participants