Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
entity system
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
12 Apr 2015 at 05:55 UTC
Updated:
26 Apr 2015 at 18:14 UTC
Jump to comment: Most recent
Comments
Comment #1
dawehnerIt absolutely it is horrible. We have been starting to use it, in more and more places, without realizing that the line numbers spit out by
\Drupal\simpletest\TestBase::getAssertionCallmakes things really hard, if you do that. In a world of data providers, we would have a data provider here.
Just in case someone is interested in data providers: theo.codes/php/improving-readability-of-phpunit-data-providers.html
Comment #2
alexpottre #1 sometimes it is correct to use an assertTHING and sometimes it is not. Generally assertTHING functions should be very very short. This one is not and it should not be an assert.
Also EntityRevisionsTest is perfectly debuggable.
However even with this change the test output will still be hard to grok because all the asserts are inside nested loops so one fail on line 92 is not actually helpful either. IMO all of this is caused by our insistence of continuing to run a test method after the first failure - which is just nuts and makes things way harder than necessary.
Committed f7740e4 and pushed to 8.0.x. Thanks!
Comment #4
chx commented> all of this is caused by our insistence of continuing to run a test method after the first failure - which is just nuts and makes things way harder than necessary.
run-tests.sh has a die-on-fail option which drupalci supports. I think getting a default on for this into the core testing suite given a core committers strong preference will be easy.