Minitest failed with status (1)

[Daniel-ltw]

Coverage report generated for Unit Tests to /app/coverage. 5422 / 26782 LOC (20.24%) covered.

rake aborted!

Command failed with status (1)

/usr/local/bundle/gems/knapsack-1.15.0/lib/knapsack/runners/minitest_runner.rb:28:in `run'

/usr/local/bundle/gems/knapsack-1.15.0/lib/tasks/knapsack_minitest.rake:5:in `block (2 levels) in <top (required)>'

/usr/local/bundle/gems/rake-12.0.0/exe/rake:27:in `<top (required)>'

/usr/local/bundle/bin/bundle:104:in `load'

/usr/local/bundle/bin/bundle:104:in `<main>'

Tasks: TOP => knapsack:minitest_run

(See full trace by running task with --trace)

Is there something wrong here?

[ArturT]

Hey @Daniel-ltw

Was the test suite executed? Did any test fail?

What version of minitest do you use? What version of Rails.

Maybe coverage report has an impact on exit code from minitest?

Do you have any non-standard configuration of minitest?

It looks like just minitest rake task failed, https://github.com/ArturT/knapsack/blob/ad3e159d6724a89d4af936abdb66fd5d3c1cdd01/lib/knapsack/runners/minitest_runner.rb#L28

Maybe try to run minitest with verbose flag to see what exactly is failing.

$ bundle exec rake "knapsack:minitest[--verbose]"

[Daniel-ltw]

I guess previously, we did not have that issue.

Will have a run with the verbose to see if that makes any difference or give us more to follow up on.

[Daniel-ltw]

I guess it is the test failures that is causing the problem, knapsack should have a way to handle Command failed with status (1)

Might need a rescue for that invoke and handle the error nicely?

[Daniel-ltw]

After 20 days, I think this step is necessary to address this issue.

https://github.com/ArturT/knapsack/blob/master/lib/knapsack/runners/minitest_runner.rb#L28

Add a rescue StandardError after this or maybe the following: https://github.com/seattlerb/minitest/blob/master/lib/minitest/test.rb#L19

This might be the appropriate way to deal with this. In test failing scenario, the error should be handle nicely and not throw an error.

[ArturT]

Hey @Daniel-ltw, sorry for late reply, I was busy with work in recent weeks.

You are thinking about something like:

def self.run(args)
  # other code
  Rake::Task[task_name].invoke
rescue StandardError
  raise
end

in order to reraise error from minitest so you will see in the output what actually failed inside of one of your minitest test files, right?

Right now the error output you added in the first post does not say much about what failed so you are hoping that by reraising error we will get the info what failed in a test file?

BTW Could you share example test file that would trigger this behavior so I could reproduce the error. Here it is an example repo with minitest test suite I use for testing https://github.com/KnapsackPro/rails-app-with-knapsack

What version of ruby and minitest do you use?

[Daniel-ltw]

@ArturT Could you try adding a failing test and see what does happen in your output?

class FailTest < ActiveSupport::TestCase
  def test_failure
    assert_equal 1, 2
  end
end

My ruby version is 2.5.0 and minitest version is 5.10.2.

[ArturT]

@Daniel-ltw What version of rails do you use?

[Daniel-ltw]

Rails 5.1

[ArturT]

I was able to reproduce it. Actually, it's expected behavior. It's just extra info about rake task output and it does not affect how minitest works. All tests were executed. The minitest produces output info which tests failed. The final exit code from knapsack is 1 which indicates that test suite failed - that's expected.

The error you that can see:

rake aborted!

Command failed with status (1)

is produced probably by the way how rake works. If you like you can do some research and see if we actually should handle the info about non zero exit code from rake. I did not find anything helpful by doing a quick search.

I tried to do:

# lib/knapsack/runners/minitest_runner.rb
        begin
          Rake::Task[task_name].invoke
        rescue StandardError => e
          puts e.inspect
        end

but there is a question what to do with the exception. Should I print it on the output to give user option to know what happened. But this is how it works already. Or I could hide the exception info but this might lead to some implicit errors hard to track. I would probably leave it as it is now.

[Daniel-ltw]

I guess in design, you are trying to focus on how knapsack should behave with

The final exit code from knapsack is 1 which indicates that test suite failed - that's expected.

but test suite failure should not mean that knapsack has fail, or should it? From a user perspective, knapsack ran fine with no issue. The failure is in the test, not within knapsack.

Anyway found this question from 2015 https://stackoverflow.com/questions/29966486/how-to-catch-rake-task-exit-status-in-another-rake-task @ciembor

[ArturT]

but test suite failure should not mean that knapsack has fail, or should it?

test suite failure should mean the knapsack failed because we want to return exit code 1 so thanks to that the CI providers will know that test suite is failing. The minitest exit code is also 1 when test suite is failing.

The question is: should I hide the exception from the minitest rake task to just make finally knapsack output nicer? I feel like I can by mistake hide too much. I'm not sure if it would be a good assumption to rescue all StandardError and hide the info that something failed. Because this way I would assume all exceptions will be related to test assertions or bugs in test suite. Theoretically, something else could be a reason for failure and we would not know about it by hiding the exception info.

[Daniel-ltw]

So in that case, we should not rescue StandardError which might encapsulate too many other errors.

rescue SystemExit => e

Like the answer in the Stackoverflow post, that might be what we might be looking for?

[ArturT]

The SystemExit is not a good option because the RuntimeError is raised instead.

The exception looks like:

#<RuntimeError: Command failed with status (1): [ruby -I"lib:test" -I"/Users/artur/.rvm/gems/ruby-2.4.2/gems/rake-11.2.2/lib" "/Users/artur/.rvm/gems/ruby-2.4.2/gems/rake-11.2.2/lib/rake/rake_test_loader.rb" "test/controllers/articles_controller_test.rb" "test/minitest/fail_test.rb" ]>

I don't see an easy option to extract info about the exit code from the exception. The info is in a text message so I would have to parse it in order to know that the exit code is 1 or I could assume the exit code should always be 1 when something went wrong. I need to exit knapsack with exit code 1 in order to make CI build to fail.

I would keep the knapsack gem as it is now because it works. If your test suite fails you already see what's wrong in the minitest output. Then you make fixes to your tests and when the test suite is green all is good again.

I don't see a new value in changing that other than just hiding the error from rake task.

[ArturT]

I reviewed it again, and I don't see an action point here. If there is any new information we should consider, please share it.

There was no activity here, so I'm closing this issue for now.