In testFilterExampleBasic(), $this->drupalPost() is called with the argument
$edit = array(
'filters[filter_time][status]' => 1,
'filters[filter_foo][status]' => 1,
);
This works, but a hapless coder may try to replace the 1 with a 0 to disable some filter or another. I suggest
$edit = array(
'filters[filter_time][status]' => TRUE,
'filters[filter_foo][status]' => TRUE,
);
See the docs for DrupalWebTestCase::drupalPost.
I do not have time now to make a patch. If you want to encourage me, then please review the one I wrote for http://drupal.org/node/1181238#comment-4608934.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | example_project-1203096-5.patch | 1.69 KB | benjifisher |
Comments
Comment #1
rfayThe real issue is: What is expected in the filter_time['status'] and filter_foo['status'] fields. This has nothing to do with drupalPost(). Since these are checkboxes, I think it's reasonable to make them booleans, although mostly PHP and PHP folks are pretty sloppy about this sort of thing.
And in PHP, a 1 works fine for TRUE and a 0 works fine for FALSE.
I don't have any objection to this change.
Comment #2
benjifisherI think there is a similar problem with xmlrpc_example.test on line 119.
Comment #3
benjifisherYes, they are checkboxes.
Maybe I was confused, but I tried using a 0 and it seemed to have no effect: the filter I was trying to disable was still enabled. Maybe the number 0 gets converted to a string '0' somewhere along the way? Just guessing here. When I used FALSE, it worked.
The docs for
drupalPost()recommend using TRUE or FALSE for checkboxes.The way the testing framework works, I should not have to make a clean install, right? I added
to filter_example.test for format 1 and
for format 2. You can see from these screenshots that the Line Break filter was enabled the first time and disabled the second time.
Comment #4
rfayYes... but: The test works perfectly... So something must have been funky in your experiment?
Are you mistaking the test for the example? The test is there to make sure we haven't broken the example. The *example* is for teaching purposes. We would never expect a "hapless coder" to be using the stuff in drupalPost(), unless they're learning how to write tests.
Comment #5
benjifisherSpeaking as a hapless coder, I can assure you that, as I write my first D7 module (a text filter), I am learning how to write tests as well as how to write a filter. I expect everything in an example project to be something that illustrates best practices: not only the .module file but also the .info and .test files.
Just because the test passes is not enough. The test has to be correctly written and test what you think it is testing. In the case of filter_example.test, I think it is all right. When I model my own test on this one, and want to disable a particular filter (the Line Break filter) I get into trouble.
Since you already said (in #1 above) that you do not object, here is a patch. It includes a chunk for xmlrpc_filter.text, as I suggested in #2.
Comment #6
rfayThis is a nice little improvement - thanks.
Committed to D7: c554b5c
D8: f4a2691
Comment #7
rfaybenjifisher's commits are mounting: http://drupal.org/node/594964/committers