Download & Extend
Issues

Test Coverage for search_simplify

Posted by BlakeLucchesi on May 10, 2008 at 9:33pm
Project:Drupal core
Version:7.x-dev
Component:search.module
Category:task
Priority:normal
Assigned:Unassigned
Status:closed (duplicate)

Issue Summary

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().

AttachmentSizeStatusTest resultOperations
search_simplify_test.patch1.97 KBIdleFAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.View details | Re-test
Login or register to post comments

Comments

#1

Posted by BlakeLucchesi on May 10, 2008 at 10:04pm

Rerolled with notes as to which test case covers which part of search_simplify and adds CJK tests.

AttachmentSizeStatusTest resultOperations
search_simplify_test.patch2.39 KBIdleFAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.View details | Re-test

#2

Posted by cfennell on May 10, 2008 at 10:08pm

Reviewed, looks good to me.

#3

Posted by dmitrig01 on May 11, 2008 at 1:35am
Status:needs review» needs work

<?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

Posted by BlakeLucchesi on May 11, 2008 at 2:33pm

Must have been tired. Thanks. Rerolled.

AttachmentSizeStatusTest resultOperations
search_simplify_test.patch2.39 KBIdleFAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.View details | Re-test

#5

Posted by douggreen on May 12, 2008 at 2:07am

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

Posted by dmitrig01 on May 16, 2008 at 4:45am

Agreed.

#7

Posted by David Lesieur on May 17, 2008 at 8:33pm

Re-rolled, switching array to initial => expected.

AttachmentSizeStatusTest resultOperations
257033_search_simplify.patch2.12 KBIdleFAILED: [[SimpleTest]]: [MySQL] Drupal installation failed.View details | Re-test

#8

Posted by David Lesieur on May 17, 2008 at 8:35pm
Status:needs work» needs review

#9

Posted by douggreen on May 20, 2008 at 12:12pm

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

Posted by douggreen on May 20, 2008 at 12:41pm
Title:Test Coverage for search_simplify();» Test Coverage for search_simplify

#11

Posted by catch on July 25, 2008 at 2:39pm
Category:feature request» task
Status:needs review» needs work

The patch applies fine and the tests run OK, but Doug's right on the comments, they need fleshing out to full sentences.

#12

Posted by jhodgdon on March 5, 2010 at 7:06pm
Status:needs work» needs review

Setting to Needs Review so the test bot can test the last patch up there. See #11, status should actually be needs work.

#13

Posted by jhodgdon on March 5, 2010 at 7:09pm
Status:needs review» needs work

#14

Posted by jhodgdon on April 29, 2010 at 11:11pm
Status:needs work» closed (duplicate)

We actually now have a comprehensive search_simplify() test case in modules/search/search.test.
#604002: Poor search support of some Unicode scripts