assertEqual() does not work with arrays
c960657 - August 24, 2008 - 21:46
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | simpletest.module |
| Category: | bug report |
| Priority: | normal |
| Assigned: | boombatower |
| Status: | closed |
Description
Calling $this->assertEqual($array1, $array2) without a third argument triggers an error. Though unusual, array comparison using == and === is allowed in PHP.
The function looks like this:
protected function assertEqual($first, $second, $message = '', $group = 'Other') {
return $this->_assert($first == $second, $message ? $message : t('%first is equal to %second', array('%first' => $first, '%second' => $second)), $group);
}In order to be used in the default message, the arrays should be converted to a string, e.g. using print_r() or similar.
#1
Messed around with print_r and as expected it works fine with none array values such as strings or numbers.
This patch adds print_r($variable, TRUE).
#2
I'm not sure it makes sense to default to a message like "1 is equal to 1".
It makes much more sense to describe the variables that were asserted rather that the values. Something like:
ERROR: assertion failed (string == "bar")ERROR: assertion failed: (glyph->num_glyphs > 0)
As seen somewhere around the big world of glib.
#3
I like the idea, but I'm afraid that would be pretty hard to implement. AFAICT the assert function has no way of knowing the variable names, at least not without parsing the source code or using similar hacks. One alternative would be to make another function, e.g.
function assertEqualVariable($varname1, $varname2), that take variable names rather than values as input. But I guess this is outside the scope of this issue.#4
This is a bug, that would be a feature request, and from experience creating a PHP debug script it is very hard to display the variable names. XDebug and the like might be able to, but once again separate issue.
#5
This is not only problematic for arrays but also objects. example
$node = node_load(array('title' => 'blabla'));$this->assertNull($node);
will produce an error. Print_r will work but will produce huge output if we are looking at node. So maybe just output 'Object is null' or something similar.
#6
This bit us too many times already (including today, during the presentation). I suggest changing all default assert messages to static texts. Patch attached.
#7
Applies, looks good. I agree.
Haven't run all tests or anything like that, just ran the block test and worked fine..
#8
Committed to CVS HEAD. Thanks! :)
#9
Automatically closed -- issue fixed for two weeks with no activity.