- Issue created by @dydave
- Merge request !27Issue #3450488: PHPUnit: Fixed Functional Tests 'HoneypotFormCacheTest'. → (Merged) created by dydave
- last update
6 months ago 29 pass - Status changed to Needs review
6 months ago 1:04am 29 May 2024 - 🇫🇷France dydave
Quick follow-up on this ticket:
A few changes were added to the current merge request MR!27 with a few comments, see above at #2.
But mostly, at this point:
Last build on MR!27: https://git.drupalcode.org/issue/honeypot-3450488/-/pipelines/184573PHPUnit job: https://git.drupalcode.org/issue/honeypot-3450488/-/jobs/1712394
The 2 issues described above seem to have been fixed and the only remaining issue is the one related with the Rules module fixed in MR:
https://git.drupalcode.org/project/rules/-/merge_requests/26We would greatly appreciate if a maintainer or someone with write permission could take a look at ticket's merge request MR!27 and let us know if there would be any more work needed.
At this point, i's really up to the maintainers:
- whether to go ahead with this merge request, although the rules test is failing,
- or whether the remaining failing test should be disabled temporarily in the same MR (so they all pass).
In any case, feel free to let us know if you have any questions or concerns on any of the changes in the merge request or this ticket in general, we would surely be happy to help.
Thanks in advance for your feedback and reviews - 🇺🇸United States tr Cascadia
For the first issue:
Yes, I think you've correctly identified it. Previously, the only way to check the header was
responseHeaderEquals()
, and NULL was specifically defined by the API to the be the return value if the header didn't exist.That has changed, and now there is a
responseHeaderDoesNotExist()
method we can use instead of testing to see if the header equals NULL.Thank you for identifying that. I will commit it after investigating to see when this new assert method was introduced - I don't want to make a change that fixes HEAD but will break older, supported versions of this module.
As far as the Rules issue, that does not need to be handled here. You've linked to the proper Rules issue - thank you. This affects the Rules Unit testing base class, which is subclassed by modules like Honeypot that provide Rules integration. But once it is fixed in Rules, it will be fixed for all modules that support Rules.
- 🇮🇳India ankitv18
Hi @TR,
I guess this MR!27 should be merge following the threads:
D.O Issue: https://www.drupal.org/project/drupal/issues/3133355 →
10.x: https://api.drupal.org/api/drupal/core%21tests%21Drupal%21Tests%21WebAss...
9.x: https://api.drupal.org/api/drupal/core%21tests%21Drupal%21Tests%21WebAss...changes made in the MR looks fine, Can we mark this RTBC or you need to time to investigate from your end?
- Status changed to Fixed
5 months ago 8:20pm 5 July 2024 - 🇺🇸United States tr Cascadia
Seems the actual core change was in #3133355: Introduce PHPUnit-compliant WebAssert::responseHeaderExists() method, and its negative → . There is no change record, but that change was introduced in early D9 so we should be good here.
Thanks for working on this. Merged.
Automatically closed - issue fixed for 2 weeks with no activity.