Test Coverage for search_simplify
BlakeLucchesi - May 10, 2008 - 21:33
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | search.module |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | needs work |
Description
This patch provides a new class for test coverage of the search_simplify function which processes text before it is inserted into the index and before it is used for searching in do_search().
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| search_simplify_test.patch | 1.97 KB | Ignored | None | None |
#1
Rerolled with notes as to which test case covers which part of search_simplify and adds CJK tests.
#2
Reviewed, looks good to me.
#3
<?php+ '10102008' => '10-10-2008',
+ '10102008' => '10/10/2008',
+ '10102008' => '10.10.2008',
?>
This won't work. array keys are unique.
Try reversing the array.
#4
Must have been tired. Thanks. Rerolled.
#5
So that we don't run into more cases in the future like dmitrig01 found, I'd switch the key and value in the array so that it's initial => expected.
#6
Agreed.
#7
Re-rolled, switching array to initial => expected.
#8
#9
Be careful when you apply this patch to make sure that you don't lose the multibyte characters. I first applied it by copying the patch from my web browser (which is what I always do), and it failed the three CJK tests. However after downloading the patch using wget, it applied and passed all tests.
I also reviewed the code, and think that it's mostly ready to go. I'm one of the worst when it comes to commenting, so I'm not exactly sure what our standard is. But if I would add anything, it's that your comments aren't fully sentences and probably should be. You might get asked to change comments like "String to lowercase." to full sentences such as "Test simplification of strings to lowercase characters."
#10
#11
The patch applies fine and the tests run OK, but Doug's right on the comments, they need fleshing out to full sentences.