Make better use of Minitest's lifecycle

[bf4]

http://blog.arkency.com/2013/06/are-we-abusing-at-exit/

Extracted from #1069

Changes are basically that we have to use Minitest.after_run because of minitest's dependencies on at_exit for when it starts to run its suite.

1. capture warnings now 'runs' entires in execute!
2. make sure we have after_run

basically, this is the meat and potatoes of the change

gem 'minitest'
require 'minitest/autorun'
if defined?(Minitest::Test)
  # Minitest 5
  # https://github.com/seattlerb/minitest/blob/e21fdda9d/lib/minitest/autorun.rb
  # https://github.com/seattlerb/minitest/blob/e21fdda9d/lib/minitest.rb#L45-L59
else
  # Minitest 4
  # https://github.com/seattlerb/minitest/blob/644a52fd0/lib/minitest/autorun.rb
  # https://github.com/seattlerb/minitest/blob/644a52fd0/lib/minitest/unit.rb#L768-L787
  # Ensure backward compatibility with Minitest 4
  Minitest = MiniTest unless defined?(Minitest)
  Minitest::Test = MiniTest::Unit::TestCase
  def Minitest.after_run(&block)
    MiniTest::Unit.after_tests(&block)
  end
end

require 'capture_warnings'
CaptureWarnings.new(fail_build = true).execute!

and

class CaptureWarnings
  def execute!
    $VERBOSE = true
    $stderr.reopen(stderr_file.path)

    Minitest.after_run do
      stderr_file.rewind
      lines = stderr_file.read.split("\n")
      stderr_file.close!
      $stderr.reopen(STDERR)
      after_tests(lines)
    end
  end

  def after_tests(lines)
    app_warnings, other_warnings = lines.partition { |line|
      line.include?(app_root) && !line.include?(bundle_dir)
    }
   #......
  end
end

[bf4]

I'm used to RSpec's test run handlers. I didn't realize that Minitest 100% relies on the at_exit hook so I pretty much need to use Minitest.after_run { } to ensure things run in the right order at the expected time

[zenspider]

It should probably be: if defined?(Minitest::Test)

[bf4]

@zenspider Thanks for that! (esp since I don't want to rely on MiniTest = Minitest unless defined?(MiniTest))

[blowmage]

If the app is using Rails 4.0 or below, and is using minitest-rails, then Minitest::Test will be defined. This is because minitest-rails uses the minitest-test gem to enable parts of the Minitest 5.0 API in MiniTest 4.0.

[bf4]

@blowmage Thanks for that. minitest-rails isn't installed in the gem, but I do call gem 'minitest' so maybe we don't need the compatibility later.

[bf4]

@blowmage n/m Rails 4.0 still limits minitest to 4.0.

But, I guess I should update the guide since newest Rails require minitest 5?

• ~> 4.2 https://github.com/rails/rails/blob/4-0-stable/activesupport/activesupport.gemspec#L26
• ~> 5.1 https://github.com/rails/rails/blob/4-1-stable/activesupport/activesupport.gemspec#L26
• ~> 5.1 https://github.com/rails/rails/blob/4-2-stable/activesupport/activesupport.gemspec#L26
• ~> 5.1 master (today)

[bf4]

ref:

• https://github.com/rails/rails/blob/9c8a5abb420ded3dce8c2a3e656948fb6430fb64/guides/bug_report_templates/generic_master.rb#L22-L23
• https://github.com/rails/rails/blob/228e3f01cb7cc5d0b51311c5c0185b878c4853cb/guides/bug_report_templates/generic_gem.rb
• https://github.com/rails/rails/pull/19818/files#diff-db6425760f8a3c5c18e1f5316a2b6c29R8

[joaomdmoura]

I confess that I was not that concerned about the at_exit usage, but indeed it seems hackish, just hope their future changes don't end up implying in further work on our side just to keep it working. I don't see how it could happen right now let's move on with it.
Why not go with the latest version of the gem instead of treating multiple versions?
You might also need to rebase it, sorry 😁

[bf4]

@joaomdmoura I just rebased... As long as we support Rails 4.0, we need to support Minitest 4.x. See #1109 (comment)

[bf4]

@joaomdmoura This fixes issues with coverage on Rails 4.0.. any objections to merging?

[NullVoxPopuli]

👍