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

SimpleCov showing incorrect coverage result when parallelize is enabled in Rails 6.0.0 beta3 #718

Open
alkesh26 opened this issue Mar 28, 2019 · 17 comments

Comments

@alkesh26
Copy link

alkesh26 commented Mar 28, 2019

Rails version: 6.0.0 beta3
Ruby version: 2.6.1

Issue
We have configured our test helper file as below

require "simplecov"

SimpleCov.start do
  add_filter "/test/"

  add_group "Models", "app/models"
end

ENV["RAILS_ENV"] ||= "test"
require_relative "../config/environment"
require "rails/test_help"

class ActiveSupport::TestCase
  parallelize(workers: :number_of_processors)
  
  fixtures :all
end

On executing rake test the coverage report is incorrect even when test cases are written of the method. So this is our user model

class User < ApplicationRecord
  devise :database_authenticatable, :registerable, :trackable,
         :recoverable, :rememberable, :validatable

  has_one_attached :avatar
  validates :email, uniqueness: true

  def admin?
    self.role == "admin"
  end

  def name
    "#{first_name} #{last_name}"
  end
end

and the test cases we have are

  def test_user_admin
    user = users :admin
    assert user.admin?
  end

  def test_user_is_not_an_admin
    user = users :albert
    assert_not user.admin?
  end

  def test_should_return_first_name_and_last_name_as_name
    user = users :albert
    assert_equal "Albert Smith", user.name
  end

But the coverage report when parallelize is enabled is as below
wrong report

and when it is commented out the test results are all green
correct report

I tried below approaches but all of them give correct report only when parallelize is commented:

  1. Simplecov with minitest Unit tests? #235 (comment)
  2. Created a .simplecov file in root directory and executed rake.
  3. https://github.com/colszowka/simplecov/issues?utf8=✓&q=is%3Aissue+is%3Aopen+parallel followed the issues and tried other approaches.

But I am still not able to get the correct coverage result when parallelize is enabled.

Many thanks in advance.

@oystersauce8
Copy link

@alkesh26 I don't have a solution for you but have link to my app and reproducible steps:

  1. Uncomment parallelize(workers: :number_of_processors)
  2. Run tests rm -rf coverage/; RAILS_ENV=test ./bin/rails test; open coverage/index.html
  3. Coverage of app/lib/watermelon/example.rb shows 0%
  4. Comment out parallelize(workers: :number_of_processors)
  5. Run tests rm -rf coverage/; RAILS_ENV=test ./bin/rails test; open coverage/index.html
  6. Coverage of app/lib/watermelon/example.rb shows 70%

@PragTob
Copy link
Collaborator

PragTob commented Jun 25, 2019

Yes this is a biggie. Sorry for taking so long to respond, I took a bit of a break from maintaining simplecov.

Making simplecov completely work with rails and its new parallelization will be a bigger undertaking I fear. PRs are welcome. I'll see if I can allocate the time for it but I'm unsure.

Thank you for reporting 💚

jdno added a commit to jdno/venja that referenced this issue Aug 11, 2019
SimpleCov does not support running tests in parallel yet, a feature
introduced only recently by Rails 6 [1]. Since our test suite is still
quite small, and we expect it to stay like this for a while, we've
disabled parallel tests.

[1]: simplecov-ruby/simplecov#718
jdno added a commit to jdno/venja that referenced this issue Aug 11, 2019
SimpleCov does not support running tests in parallel yet, a feature
introduced only recently by Rails 6 [1]. Since our test suite is still
quite small, and we expect it to stay like this for a while, we've
disabled parallel tests.

[1]: simplecov-ruby/simplecov#718
jdno added a commit to jdno/venja that referenced this issue Aug 11, 2019
SimpleCov does not support running tests in parallel yet, a feature
introduced only recently by Rails 6 [1]. Since our test suite is still
quite small, and we expect it to stay like this for a while, we've
disabled parallel tests.

[1]: simplecov-ruby/simplecov#718
@bbascarevic
Copy link

bbascarevic commented Sep 19, 2019

As a workaround we can piggy-back on the already existing feature of result merging, we just need to trick SimpleCov that each parallel run is another Command. We set SimpleCov.use_merging to true in the root process, and then in each fork we change the command name so forks would not overwrite each others' results.

# test_helper.rb

require 'simplecov'

SimpleCov.use_merging(true)  # Important!
SimpleCov.start('rails') do
  ... # Whatever you use here...
end

module ActiveSupport
  class TestCase
    parallelize

    parallelize_setup do |worker|
      SimpleCov.command_name("#{SimpleCov.command_name}-#{worker}")
      SimpleCov.pid = Process.pid
      SimpleCov.at_exit {} # This will suppress formatters running at the end of each fork
    end
    # ...
  end
end

@chrisdebruin
Copy link

@bbascarevic-tti doesn't work for me, Are you running test with PARALLEL_WORKERS=x ?
I' am using parallelize(workers: :number_of_processors) and it reports wrong coverage.

@brandoncordell
Copy link

@bbascarevic-tti @chrisdebruin This doesn't work for me either. I'm also using parallelize(workers: :number_of_processors).

@michaelherold
Copy link

The solution by @bbascarevic-tti is almost there. Here's what I ended up with; it seems to be working as expected:

if ENV['COVERAGE']
  require 'simplecov'

  SimpleCov.start 'rails'
end

ENV['RAILS_ENV'] ||= 'test'
require_relative '../config/environment'
require 'rails/test_help'

module ActiveSupport
  class TestCase
    # Run tests in parallel with specified workers
    parallelize(workers: :number_of_processors)

    if ENV['COVERAGE']
      parallelize_setup do |worker|
        SimpleCov.command_name "#{SimpleCov.command_name}-#{worker}"
      end

      parallelize_teardown do |worker|
        SimpleCov.result
      end
    end

    # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order.
    fixtures :all

    # Add more helper methods to be used by all tests here...
  end
end

There are a few changes from @bbascarevic-tti's solution:

  1. Do not set SimpleCov.pid to Process.pid in the child process. This causes SimpleCov to report a failure for every single child process because SimpleCov then thinks that the child process is the coordinator.
  2. Do not specify SimpleCov.use_merging since it defaults to true.
  3. Don't bother wiping out the SimpleCov.at_exit hook - these are not run in the workers anyway.
  4. Set a parallelize_teardown hook to call SimpleCov.result. This complex method generates the result and, if SimpleCov.use_merging is true (which it is by default), stores the result for later merging. This allows the results of all workers to be merged into a single result.

Here's an example output:

↪ rm -rf coverage/ && DISABLE_SPRING=1 COVERAGE=1 bin/rails test
Run options: --seed 52156

# Running:

......................

Finished in 0.393015s, 55.9775 runs/s, 101.7772 assertions/s.
22 runs, 40 assertions, 0 failures, 0 errors, 0 skips
Coverage report generated for MiniTest, MiniTest-0, MiniTest-1, MiniTest-2, MiniTest-3 to /home/herold/project/coverage. 58 / 148 LOC (39.19%) covered.

I've run the wipe / re-run without Spring many times and receive a consistent result so I don't believe there are any race conditions in the process model. When you mix in Spring, you get the lovely problems inherent in such a venture, so YMMV if you choose to try it with Spring.

Threads

Note that I've only been able to produce completely consistent results using process parallelization. When I tested out with: :threads, I noticed that the coverage numbers would non-deterministically jump between 58 / 148 and 49 / 148. There were also errors due to thread deadlocks in the database too though, so I think threads on MRI just aren't fully baked (which is fine since you don't get true parallelism anyway).

Next Steps

I haven't been able to figure out a way to upstream this workflow into SimpleCov, so if someone has any bright ideas, please feel free to do that! The way parallel_tests sets the environment variables and gives you access to both the current worker ID and the total number of workers makes sense. Perhaps Rails could use similar functionality to pass the total number of workers into the parallelize_setup and parallelize_teardown hooks?

@brandoncordell
Copy link

brandoncordell commented Oct 4, 2019

@michaelherold I think you're close! I'm running into an issue though and maybe it's just general SimpleCov config and not due to parallelize?

Regular tests

rails test
Running via Spring preloader in process 77422
Run options: --seed 42107

# Running:

.........................

Finished in 12.725153s, 1.9646 runs/s, 2.9076 assertions/s.
25 runs, 37 assertions, 0 failures, 0 errors, 0 skips
Coverage report generated for MiniTest, MiniTest-0, MiniTest-1 to  
/Users/Bcordell/Code/<redacted>/coverage. 177 / 232 LOC (76.29%) covered.

System tests run right afterwards

rails test:system                                                                                        

# Running:

Capybara starting Puma...
* Version 3.12.1 , codename: Llamas in Pajamas
* Min threads: 0, max threads: 4
* Listening on tcp://127.0.0.1:63394
Capybara starting Puma...
* Version 3.12.1 , codename: Llamas in Pajamas
* Min threads: 0, max threads: 4
* Listening on tcp://127.0.0.1:63392
..

Finished in 14.470213s, 0.1382 runs/s, 0.0691 assertions/s.
2 runs, 1 assertions, 0 failures, 0 errors, 0 skips
Coverage report generated for MiniTest, MiniTest-0, MiniTest-1 to 
/Users/Bcordell/Code/<redacted>/coverage. 122 / 293 LOC (41.64%) covered.

Regular tests again run right after the system tests

Regular tests

rails test
Running via Spring preloader in process 77422
Run options: --seed 42107

# Running:

.........................

Finished in 12.725153s, 1.9646 runs/s, 2.9076 assertions/s.
25 runs, 37 assertions, 0 failures, 0 errors, 0 skips
Coverage report generated for MiniTest, MiniTest-0, MiniTest-1 to  
/Users/Bcordell/Code/<redacted>/coverage. 177 / 232 LOC (76.29%) covered.

It seems like it's not merging my system tests when I run rails test:system.

Edit

I just had to put SimpleCov.command = 'test:system' inside of ApplicationSystemTestCase and the fix is working perfectly!

@PragTob
Copy link
Collaborator

PragTob commented Oct 5, 2019

Hey everyone,

thanks all of you for your work on this and exchanging ideas helping each other and laying important ground work 👏

I don't have a lot of time to invest into this right now (sadly), however I submitted a simplecov project to RubyTogether and this is on the list of things to work on/fix should the project get accepted.

@PragTob
Copy link
Collaborator

PragTob commented Dec 3, 2019

And as a small update, I did get the ruby together fund and hence an improved rails support especially its new test parallelization is towards the top of the list (after branch coverage and some friends though)

miyohide added a commit to miyohide/my_rails_template that referenced this issue May 23, 2020
evazion added a commit to danbooru/danbooru that referenced this issue Jun 11, 2020
* Fix simplecov clobbering test coverage reports when using parallel tests.
* Generate coverage reports by default (remove $SIMPLECOV flag).
* Store coverage reports in tmp/coverage/ instead of coverage/.
* Enable branch coverage.

ref: github.com /simplecov-ruby/simplecov/issues/718#issuecomment-538201587
@AxelTheGerman
Copy link

AxelTheGerman commented Jun 21, 2020

@PragTob what's the status on this one? Disabling the parallelize feature of Rails is definitely a workaround but would be great to have simplecov work with it - looks like some of the proposals above might be quite close to getting things working properly.

Edit: while disabling parrallelize seems to work when I just run rails test it definitely doesn't when I want to run both, system and regular, test suites together via rails test:system test 😞

@PragTob
Copy link
Collaborator

PragTob commented Jun 23, 2020

@AxelTheGerman hey, well - in the end as it's always the case with projects it didn't make the cut in the end. There were more branch coverage issues to iron out (and even more now that I finally used it in a project myself). So it's kinda a question of "when I get to it". Truth be told, due to other circumstances I'm not spending too much time on OSS these days. I might bite down some time during a vacation and do it, however, I think I'd first work on polishing existing features (branch coverage...) and bugs. As I'm sure, the parallelize implementation will bring many more fun bugs for me to deal with. Such as #815. Not the answer you were looking for I wager, but sadly the honest one I can give :)

@AxelTheGerman
Copy link

@PragTob thanks for all the awesome work you do!

I also agree on making existing features robust first - as well as taking care of bugs and tech debt.

I am fine without parallelize support but for now I haven't figured out how to even turn it off properly. If there is a workaround to get rid of parallelization in a Rails 6 app that'd be great. All I want is running regular tests and system tests with combined coverage - which should be possible with rails test:system test

I have to check some of my other projects and see what might be different. Will report back when I figure it out :)

@AxelTheGerman
Copy link

I think the problem was either that I still had parallelize(workers: 1) in my test setup OR that I actually didn't have any system tests yet.
As soon as I removed parallelize completely and added a working spec, the test coverage is working fine for rails test:system test 🎉

@viral810
Copy link

viral810 commented Jul 9, 2021

Omit loading spring from bin/rails and bin/rake from Rails 6 application. Loading spring server has his side effects

# bin/rails

#!/usr/bin/env ruby
# load File.expand_path('spring', __dir__)
...

universato added a commit to universato/BrainDriller that referenced this issue Jul 10, 2021
simplecov-ruby/simplecov#718
Rails6のパラレルテスト対策
@joshmfrankel
Copy link

This may be common knowledge, but make sure you're also eager_loading your testing environment. I followed the above advice, but still had some accuracy issues until I discovered this. Just run with CI=true bin/rails test:all

# config/environments/test.rb
Rails.application.configure do
  # many more configs...
  
  # Use the CI EnvVar to control this
  config.eager_load = ENV["CI"].present?
  
  # some more configs...
end

@searls
Copy link

searls commented Nov 7, 2022

Any interest in maybe documenting @michaelherold's (excellent! working!) approach in the README, until/unless built-in support for Rails parallelization is added?

Now that Rails will only start parallelizing tests once you hit 50 of them, the fact coverage will suddenly go from correct to near-0% is really confusing and takes time to piece together by searching issues in the repo. (The optimization is totally valid, but when a working initial setup stops working suddenly it can be disorienting.)

@matteeyah
Copy link

For some reason, I still can't get SimpleCov to report correct percentages - e.g. it's reporting 0% coverage on ApplicationController, but properly calculates coverage for most other controllers.


This is our test/test_helper.rb.

# frozen_string_literal: true

if ENV['CI']
  require 'simplecov'
  require 'simplecov-cobertura'
  require 'minitest/reporters'

  SimpleCov.start 'rails' do
    command_name = ENV.fetch('CI_JOB_NAME_SLUG', nil) # rubocop:disable Lint/UselessAssignment

    add_filter '/app/admin'
    add_filter 'vendor'

    add_group 'Policies', 'app/policies'

    formatter SimpleCov::Formatter::CoberturaFormatter
  end

  Minitest::Reporters.use!([
                             Minitest::Reporters::DefaultReporter.new,
                             Minitest::Reporters::JUnitReporter.new('test/reports', true, single_file: true)
                           ])
end

ENV['RAILS_ENV'] ||= 'test'
require_relative '../config/environment'
require 'rails/test_help'

require 'webmock/minitest'
require 'sidekiq/testing'

module ActiveSupport
  class TestCase
    # Run tests in parallel with specified workers
    parallelize(workers: :number_of_processors)

    # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order.
    fixtures :all

    if ENV['CI']
      parallelize_setup { |worker| SimpleCov.command_name "#{SimpleCov.command_name}-#{worker}" }
      parallelize_teardown { |_| SimpleCov.result }
    end

    teardown do
      Redis.new(url: ENV.fetch('REDIS_URL', 'redis://localhost:6379/1')).flushdb
    end

    # Add more helper methods to be used by all tests here...
  end
end

# Disable external requests
WebMock.disable_net_connect!(
  allow_localhost: true,
  allow: ['https://googlechromelabs.github.io', 'https://edgedl.me.gvt1.com']
)

# Use OmniAuth in test mode
OmniAuth.config.test_mode = true

Sidekiq::Testing.fake!

This is how we collate the results

namespace :coverage do
  task report: :environment do
    require 'simplecov'

    SimpleCov.collate Dir['coverage/.resultset*.json'], 'rails' do
      formatter SimpleCov::Formatter::HTMLFormatter
    end
  end
end

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

No branches or pull requests