simplecov icon indicating copy to clipboard operation
simplecov copied to clipboard

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

Open alkesh26 opened this issue 5 years ago • 17 comments

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. https://github.com/colszowka/simplecov/issues/235#issuecomment-22271831
  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.

alkesh26 avatar Mar 28 '19 20:03 alkesh26

@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%

oystersauce8 avatar Apr 20 '19 18:04 oystersauce8

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 :green_heart:

PragTob avatar Jun 25 '19 09:06 PragTob

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

bbascarevic avatar Sep 19 '19 14:09 bbascarevic

@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.

chrisdebruin avatar Sep 23 '19 11:09 chrisdebruin

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

brandoncordell avatar Oct 02 '19 17:10 brandoncordell

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?

michaelherold avatar Oct 04 '19 02:10 michaelherold

@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!

brandoncordell avatar Oct 04 '19 14:10 brandoncordell

Hey everyone,

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

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 avatar Oct 05 '19 14:10 PragTob

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)

PragTob avatar Dec 03 '19 09:12 PragTob

@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 :disappointed:

AxelTheGerman avatar Jun 21 '20 22:06 AxelTheGerman

@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 :)

PragTob avatar Jun 23 '20 18:06 PragTob

@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 avatar Jun 23 '20 20:06 AxelTheGerman

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 :tada:

AxelTheGerman avatar Jun 24 '20 04:06 AxelTheGerman

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__)
...

viral810 avatar Jul 09 '21 16:07 viral810

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

joshmfrankel avatar Mar 05 '22 15:03 joshmfrankel

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.)

searls avatar Nov 07 '22 15:11 searls

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

matteeyah avatar Jan 12 '24 23:01 matteeyah