Problem/Motivation

Previously we fixed 3298558 issue.

I've tried to run tests but unfortunately I spent lot of time without success.
I think my the previous patch is broke a kernel test.

tests/src/Kernel/GroupMembershipRequestTest::testRequestWithMemberAsUser

$this->assertTrue(in_array('User "%name" is already a member of group', $messages), 'The validation of group membership request is found.');

We should change %name to account->getDisplayName()

Please run your tests if possible, fix and commit patch for it.
Thank you!

Issue fork grequest-3298919

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

kecsot created an issue. See original summary.

kecsot’s picture

kecsot
he/him
commented
Title: Missing a parameter from a test » Missing account's displayName from testRequestWithMemberAsUser

LOBsTerr made their first commit to this issue’s fork.

LOBsTerr opened merge request !14

lobsterr’s picture

lobsterr commented
Status: Active » Needs review

Unfortunately, I couldn't reproduce your error in the test. Probably, it is related to some local configuration, but some tests were broken and I fixed them and more tests to cover recent changes.

kecsot’s picture

kecsot
he/him
commented

Thank you for those fixes.

Sorry. I made mistake.
Test will may fail on developer branch or if you apply 3298558 Issue’s patch to 1.0.4 version.

lobsterr’s picture

lobsterr commented

@kecsot, when we create a new fork it is always created against dev branch. This is curent case. As you see the patch is already applied in dev branch

https://git.drupalcode.org/project/grequest/-/blob/1.0.x/src/Plugin/Vali...

and it also in fork with tests

https://git.drupalcode.org/issue/grequest-3298919/-/blob/3298919-missing...

Have you tried to run the tests from MR ?

BTW - I run the test always in clean Drupal

kecsot’s picture

kecsot
he/him
commented

As I said I can't run tests. I just created this issue to check it if you can.
Got it! I understand this test is green.

Test use $violation->getMessage()->getUntranslatedString() function which returns
"User "%name" is already a member of group".

I just suggest to extend the test to cover 3298558 issue, if possible in Kernel test.

I'm done with this issue. Thanks and sorry for waste time!

  • LOBsTerr committed aa18305 on 1.0.x 
    Issue #3298919 by LOBsTerr: Missing account's displayName from...
lobsterr’s picture

lobsterr commented
Status: Needs review » Fixed

no problem at all, I have update the tests thanks to you. Thank for helping me to make this module better

Status: Fixed » Closed (fixed)

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