The Drupal\views\Tests\Plugin\FilterTest::testFilterQuery contains a test that checks for 4 results, but the comment and test message refer to no results. This patch fixes that.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jcnventura’s picture

pguillard’s picture

Status: Active » Needs review

+1

dawehner’s picture

Status: Needs review » Reviewed & tested by the community

Seems alright for me. In some world we could fix things and use Safemarkup::format(), if possible.

webchick’s picture

Status: Reviewed & tested by the community » Needs work

Hm. Seems like we lost some valuable context in that documentation about why we are checking for 4 records.

jcnventura’s picture

Status: Needs work » Needs review
FileSize
1.51 KB

It was not a case of losing valuable information, but yes, the comment can be a lot more verbose on what's going on.

Anonymous’s picture

Status: Needs review » Needs work

Looks good. A nitpick though:

+++ b/core/modules/views/src/Tests/Plugin/FilterTest.php
@@ -88,7 +88,8 @@ public function testFilterQuery() {
+    // Check that we have a single element, as a result of applying the
+    // '= John' filter.

'= John' should go up a line as to not violate or coding standards.

jcnventura’s picture

Status: Needs work » Needs review

The 80-char max? I believe doing what you ask would actually break it...
Is there an obscure coding standard that I'm not aware here?

Anonymous’s picture

Issue summary: View changes
Status: Needs review » Needs work
FileSize
56.39 KB

The 80 chars indeed. In the screenshot below, I edited the html source of the dreditor view to demonstrate how I think it should look like.

pguillard’s picture

Actually we are exactly on the 80 chars limit !

jcnventura’s picture

Status: Needs work » Needs review
Anonymous’s picture

Status: Needs review » Reviewed & tested by the community

Yes, indeed :)

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: testfilterquery_wrong_msg-2471609-9.patch, failed testing.

jcnventura’s picture

Status: Needs work » Reviewed & tested by the community

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 9: testfilterquery_wrong_msg-2471609-9.patch, failed testing.

pguillard’s picture

I raised the re-testing because tests are positive

pguillard’s picture

Status: Needs work » Reviewed & tested by the community
alexpott’s picture

Status: Reviewed & tested by the community » Fixed

Committed 836d463 and pushed to 8.0.x. Thanks!

  • alexpott committed 836d463 on 8.0.x
    Issue #2471609 by jcnventura, pguillard: FilterTest testFilterQuery...

Status: Fixed » Closed (fixed)

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