Problem/Motivation

Some tests are failing - please community help us fixing them and add further important tests to improve this module!

See https://www.drupal.org/pift-ci-job/2159155

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork restrict_ip-3229792

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

Anybody created an issue. See original summary.

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

cskeers opened merge request !6

cskeers’s picture

cskeers commented

I think I may have bungled opening the MR, but I attempted to make some updates to the tests in a branch in order to get them mostly working.

Many tests still fail with these errors unless the MR from https://www.drupal.org/project/restrict_ip/issues/2957482 is added as well, but on the plus side, making some of these test updates makes it so a separate test for that issue is not strictly necessary!

joewhitsitt’s picture

joewhitsitt
Location Iowa
commented

I don't know how to fix it but here is the diff from cskeers work: https://git.drupalcode.org/issue/restrict_ip-3229792/-/compare/3229792-f...

cskeers opened merge request !7

cskeers closed merge request !6

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Version: 3.x-dev » 4.x-dev
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

Thanks @cskeers, we'll have a look and will also commit the other issue, presumably today.

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Assigned: Unassigned » grevil
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Active » Needs work
anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented

@Grevil: Still 3 broken tests left in 4.x - please fix them, if they are easy to resolve. Thanks :)

Anybody closed merge request !7

Anybody reopened merge request !7

Anybody closed merge request !7

Anybody opened merge request !11

Anybody closed merge request !11

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

Grevil opened merge request !12

Grevil reopened merge request !6

grevil’s picture

grevil commented
Status: Needs work » Needs review

1 Test still fails, and I don't understand why. I couldn't replicate the issue, so I disabled the test and created a follow-up issue here: #3328778: Reimplement testWhitelistedIpAddresses.

All other tests should go green now! :)

anybody’s picture

anybody
Primary language German
Location Porta Westfalica
commented
Status: Needs review » Reviewed & tested by the community

Perfect, thank you for fixing this @Grevil! That was quite important.

grevil’s picture

grevil commented

All green! Merging

  • Grevil committed be066e8f on 4.x 
    Issue #3229792: Fix broken tests and improve test coverage

  • Grevil committed a78dd8cb on 3.x authored by cskeers 
    Issue #3229792: Fix broken tests and improve test coverage
grevil’s picture

grevil commented
Status: Reviewed & tested by the community » Fixed

Thanks all!

Status: Fixed » Closed (fixed)

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