Task to convert core/modules/search/lib/Drupal/search/Tests/SearchExpressionInsertExtractTest.php to phpunit.

See #1938068: Convert UnitTestBase to PHPUnit.

Files: 
CommentFileSizeAuthor
#11 interdiff.txt889 bytesjhedstrom
#11 search-phpunit-2002190-11.patch10.36 KBjhedstrom
PASSED: [[SimpleTest]]: [MySQL] 56,009 pass(es).
[ View ]
#7 search_expression-2002190-7.patch10.37 KBdawehner
FAILED: [[SimpleTest]]: [MySQL] 55,886 pass(es), 6 fail(s), and 0 exception(s).
[ View ]
#7 interdiff.txt1.95 KBdawehner
#5 drupal-2002190-5.patch10.37 KBdawehner
PASSED: [[SimpleTest]]: [MySQL] 55,918 pass(es).
[ View ]
#1 convert-search-phpunit-2002190-01.patch2.92 KBjhedstrom
PASSED: [[SimpleTest]]: [MySQL] 55,795 pass(es).
[ View ]

Comments

jhedstrom’s picture

Status:Active» Needs review
StatusFileSize
new2.92 KB
PASSED: [[SimpleTest]]: [MySQL] 55,795 pass(es).
[ View ]

Again fairly simple.

dawehner’s picture

It would be great to convert this to dataProviders

$cases = array(
array('foo', 'bar', 'foo:bar', 'bar'), // Normal case.
array('foo', NULL, '', NULL), // Empty value: shouldn't insert.
array('foo', ' ', 'foo:', ''), // Space as value: should insert but retrieve empty string.
array('foo', '', 'foo:', ''), // Empty string as value: should insert but retrieve empty string.
array('foo', '0', 'foo:0', '0'), // String zero as value: should insert.
array('foo', 0, 'foo:0', '0'), // Numeric zero as value: should insert.
);

PS: It might make sense to put this static stuff into classes.

dawehner’s picture

Status:Needs review» Needs work

.

ParisLiakos’s picture

yeah we should first convert procedural code to OOP and its test to PHPUnit first:) not the other way around

dawehner’s picture

Status:Needs work» Needs review
StatusFileSize
new10.37 KB
PASSED: [[SimpleTest]]: [MySQL] 55,918 pass(es).
[ View ]

Instead of relying on pure static methods I think we have clearly an object here: the search expression.

ParisLiakos’s picture

besides the assertion message everything looks good. if we want to keep them we should reverse them, so they make some sense when phpunit displays them (only upon failure)

dawehner’s picture

Title:Convert core/modules/search/lib/Drupal/search/Tests/SearchExpressionInsertExtractTest.php to phpunit» Convert core/modules/search/lib/Drupal/search/Tests/SearchExpressionTest.php to phpunit
StatusFileSize
new1.95 KB
new10.37 KB
FAILED: [[SimpleTest]]: [MySQL] 55,886 pass(es), 6 fail(s), and 0 exception(s).
[ View ]

The message should be also inverted, as it is just displayed on failure.

Status:Needs review» Needs work
Issue tags:-phpunit

The last submitted patch, search_expression-2002190-7.patch, failed testing.

ParisLiakos’s picture

Status:Needs work» Needs review

#7: search_expression-2002190-7.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+phpunit

The last submitted patch, search_expression-2002190-7.patch, failed testing.

jhedstrom’s picture

Status:Needs work» Needs review
StatusFileSize
new10.36 KB
PASSED: [[SimpleTest]]: [MySQL] 56,009 pass(es).
[ View ]
new889 bytes

There was a typo in #7, where both arguments were passed to trim(). This version removes trim() altogether as I didn't see the need for it.

dawehner’s picture

Oh thanks! I fear that I can't RTBC it.

ParisLiakos’s picture

Status:Needs review» Reviewed & tested by the community

i love this patch, thanks!

jhedstrom’s picture

Regarding the inverting of the messages, my understanding is the printed message should reflect the expected outcome, not an error message, in which case we'd need to flip these messages back to the original text.

ParisLiakos’s picture

this is the case for simpletest not PHPUnit. PHPUnit only displays those messages upon failure.
assertEquals example:

Reports an error identified by $message if the two variables $expected and $actual are not equal.

alexpott’s picture

Status:Reviewed & tested by the community» Fixed

Committed 7b0039e and pushed to 8.x. Thanks!

Automatically closed -- issue fixed for 2 weeks with no activity.

jhedstrom’s picture