CommentFileSizeAuthor
#7 Module_override_of_result_set_not_working-2592875-7.patch1.65 KBJordiK
#6 2592875-6.patch716 bytesTR
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jhedstrom created an issue. See original summary.

pedrorocha’s picture

pedrorocha CreditAttribution: pedrorocha commented
Status: Active » Postponed (maintainer needs more info)

Hi @jhedstrom, it wasn't clear what happened to you. Can you give more details?

jhedstrom’s picture

jhedstrom
Primary language English
Location Portland, OR
CreditAttribution: jhedstrom at Phase2 commented

These are the 2 commented out tests from the patch in #2592867: Get tests passing and enable automated tests for 8.x. The tests are looking for the ability of other modules to change result sets. Rather than try and fix it there, I opened this separate issue.

pedrorocha’s picture

pedrorocha CreditAttribution: pedrorocha commented
Status: Postponed (maintainer needs more info) » Active

Ok. I'll keep it open for now, as no one is using it yet and we have many things to do to have the basic features working. BTW, if you have more ideas of tests to do, please let me know or send a patch. I just created the basic stuff there to start the port, but it's good to map more tests to have a good code coverage.

pedrorocha’s picture

pedrorocha CreditAttribution: pedrorocha commented

I'm working on Fivestar port to D8(currently on https://github.com/pedrorocha-net/fivestar) and this problem arose for me too. I'm going to work on it soon.

TR’s picture

TR CreditAttribution: TR commented
Status: Active » Needs review
FileSize
2592875-6.patch716 bytes

I had to look through all the test source code to find this - it wasn't described in the issue you linked to. There should really be a @see after the @todo in the test so that in case anyone wants to work on this they can find it.

This patch just adds the @see, it doesn't fix the test.

JordiK’s picture

JordiK CreditAttribution: JordiK commented
FileSize
Module_override_of_result_set_not_working-2592875-7.patch1.65 KB

Small things are hard to spot.

The two lines, which failed the tests are checking whether the custom result function plugin Zebra is registered.

The problem was misspelled plugin annotation in votingapi\tests\modules\votingapi_test\src\Plugin\VoteResultFunction\Zebra.php
It read @VoteResult, but should have been named @VoteResultFunction instead.

I removed the comments of both asserts in question - now the test runs without errors.

TR’s picture

TR CreditAttribution: TR commented
Status: Needs review » Reviewed & tested by the community

WOW. Really nice work @JordiK. That has been wrong for 5 years and no one spotted it!

  • pifagor committed 443883e on 8.x-3.x authored by JordiK 
    Issue #2592875 by TR, JordiK, pedrorocha, jhedstrom, pifagor: Module...
pifagor’s picture

pifagor
Location 🇺🇦 Rivne
CreditAttribution: pifagor at Drupal Ukraine Community, GOLEMS GABB for Drupal Ukraine Community, GOLEMS GABB commented
Status: Reviewed & tested by the community » Fixed

good catch )

Status: Fixed » Closed (fixed)

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