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

Unit: Errors in teardown are not recorded #221

Closed

Conversation

randycoulman
Copy link

When an error or assertion failure occurs in a teardown method, it is not recorded correctly. That is, record has been sent with the passing test information before teardown is sent, and no further information is recorded after that. This doesn't affect the standard runner, because it relies on puke to do its job. But it affects other runners that rely on record instead, such as minitest-reporters.

This pull request consists of three commits:

  1. Add test coverage around the current incorrect behavior of record. There may be a better way to write these tests.
  2. Modify the relevant test to have the desired new behavior.
  3. (Optional) If an error/assertion failure is raised in both the test body and teardown, record the one from the test body.

See also the related issue I filed for minitest-reporters.

@zenspider
Copy link
Collaborator

  1. Your tests look good, tho I can probably clean them up a bit to make them more readable, with one exception described below.
  2. ...yup
  3. I think I object to this. It should record both IMO. As such, I think the implementation side of this patch can be whittled down to a simple record in the rescue Exception block. I'm not sure I entirely understand your usecase tho, so I can be convinced otherwise.

Any objections? It passes all but one of your scenarios, which I've renamed to test_record_error_in_test_and_teardown. Both that and the test_record_error_teardown case needed to be slightly modified to reflect that record was called twice.

@zenspider
Copy link
Collaborator

OK. I went a bit apeshit overhauling the tests but the essence is exactly the same (minus my objection above).

Please see my changes in c4a4d92 and review. Thanks!

@zenspider zenspider closed this Jan 15, 2013
@randycoulman
Copy link
Author

I won't have time to review your changes in more detail until later tonight, but I wanted to comment on the change you made to #3. Since I haven't looked at the code, I might have the details wrong below.

I thought of doing it this way (extra call to record), and ultimately, I think I like it better, because it gives clients a way to have all of the same information that puke gets. Consistency is a good thing.

However, if I understand your explanation correctly, you've changed the semantics of record so that it might be called more than once per test. I know that this will break minitest-reporters the way it is currently written, and I'm not sure what other clients of that API will be affected. So, it might be necessary to bump the major version number if you go this way, and probably a doc update for record would be a good idea.

@randycoulman
Copy link
Author

I've had a chance to look at the code now. I really like what you did with the tests. Much better than what I had. Very nice.

Other than that, my earlier comment about the change in semantics stands. I like the new semantics better, because it gives clients the opportunity to report both errors. So, as long as you're OK with a potential downstream surprise, that's fine with me. I'll probably put together a pull request for minitest-reporters to deal with the new semantics.

@bogn
Copy link

bogn commented Feb 12, 2013

This is probably a separate issue because in this case the errors occur in the setup call, but apart from that I'm seeing similar behavior to the original topic with minitest 4.6.0 and various versions of minitest-reporters including the latest (0.14.7). When I run functionals the summary reports 7 errors (expected errors for missing functionality on my side) but not a single stacktrace is printed. Instead the following can be found at the end of the output:

rake aborted!
Command failed with status (7): [ruby -I"lib:test" -I"/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib" "/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/rake_test_loader.rb" "test/functional/**/*_test.rb" ]
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/testtask.rb:104:in `block (3 levels) in define'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/file_utils.rb:45:in `call'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/file_utils.rb:45:in `sh'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/file_utils_ext.rb:40:in `sh'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/file_utils.rb:82:in `ruby'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/file_utils_ext.rb:40:in `ruby'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/testtask.rb:100:in `block (2 levels) in define'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/file_utils_ext.rb:61:in `verbose'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/testtask.rb:98:in `block in define'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/task.rb:228:in `call'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/task.rb:228:in `block in execute'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/task.rb:223:in `each'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/task.rb:223:in `execute'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/task.rb:166:in `block in invoke_with_call_chain'
/home/bogn/.rvm/rubies/ruby-1.9.3-p286/lib/ruby/1.9.1/monitor.rb:211:in `mon_synchronize'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/task.rb:159:in `invoke_with_call_chain'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/task.rb:152:in `invoke'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:143:in `invoke_task'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:101:in `block (2 levels) in top_level'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:101:in `each'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:101:in `block in top_level'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:110:in `run_with_threads'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:95:in `top_level'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:73:in `block in run'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:160:in `standard_exception_handling'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:70:in `run'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/bin/rake:33:in `<top (required)>'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/bin/rake:19:in `load'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/bin/rake:19:in `<top (required)>'
-e:1:in `load'
-e:1:in `<main>'
Tasks: TOP => test:functionals

Process finished with exit code 1

When I set 'minitest', '< 4.2.0' (resulting in 4.1.0) in the Gemfile the exceptions get printed and the reporter works as expected in RubyMine 5.0.

@zenspider
Copy link
Collaborator

Please don't line wrap the terminal output. It's impossible to read

On Feb 12, 2013, at 3:14, bogn notifications@github.com wrote:

This is probably a separate issue, but I'm seeing similar behavior to the original topic with minitest 4.6.0 and various versions of minitest-reporters including the latest (0.14.7). When I run functionals the summary reports 7 errors (expected errors for missing functionality on my side) but not a single stacktrace is printed. Instead this is the output:

rake aborted!
Command failed with status (7): [ruby -I"lib:test" -I"/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib" "/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/rake_test_loader.rb" "test/functional/*_/__test.rb" ]
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/testtask.rb:104:in block (3 levels) in define' /home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/file_utils.rb:45:incall'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/file_utils.rb:45:in sh' /home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/file_utils_ext.rb:40:insh'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/file_utils.rb:82:in ruby' /home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/file_utils_ext.rb:40:inruby'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/testtask.rb:100:in block (2 levels) in define' /home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/file_utils_ext.rb:61:inverbose'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/testtask.rb:98:in block in define' /home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/task.rb:228:incall'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/task.rb:228:in block in execute' /home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/task.rb:223:ineach'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/task.rb:223:in execute' /home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/task.rb:166:inblock in invoke_with_call_chain'
/home/bogn/.rvm/rubies/ruby-1.9.3-p286/lib/ruby/1.9.1/monitor.rb:211:in mon_synchronize' /home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/task.rb:159:ininvoke_with_call_chain'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/task.rb:152:in invoke' /home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:143:ininvoke_task'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:101:in block (2 levels) in top_level' /home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:101:ineach'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:101:in block in top_level' /home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:110:inrun_with_threads'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:95:in top_level' /home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:73:inblock in run'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:160:in standard_exception_handling' /home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/lib/rake/application.rb:70:inrun'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/gems/rake-10.0.3/bin/rake:33:in <top (required)>' /home/bogn/.rvm/gems/ruby-1.9.3-p286/bin/rake:19:inload'
/home/bogn/.rvm/gems/ruby-1.9.3-p286/bin/rake:19:in <top (required)>' -e:1:inload'
-e:1:in `

'
Tasks: TOP => test:functionals

Process finished with exit code 1
When I set 'minitest', '< 4.2.0' (resulting in 4.1.0) in the Gemfile the exceptions get printed and the reporter works as expected in RubyMine 5.0.


Reply to this email directly or view it on GitHub.

@minitest minitest locked and limited conversation to collaborators May 17, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants