Problem/Motivation

PHPunit Next Major pipeline: https://git.drupalcode.org/issue/feeds-3430449/-/jobs/1869217#L6303

  • As book is now contributed module we need to make changes in our tests as it starts failing for current and previous minor if I put in the require-dev section
  • Laminas/laminas-feed ExtensionManagerInterface is also deprecated which is implemented in ZfExtensionManagerSfContainer and SyndicationParser (Needs extra effort here)
  • DataProviders should be static
  • Fix all deprecated phpunit methods like returnValue(), returnValueMap(), returnCallback() and many more.

Proposed resolution

  • Add drupal/book-book: "^1" in the require-dev section of composer json.
  • Fix all phpunit deprecation methods

Issue fork feeds-3454788

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

ankitv18 created an issue. See original summary.

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

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

megachriz opened merge request !190

megachriz’s picture

megachriz
Primary language Dutch
commented

The tests of the previous minor fail because of a critical bug in the Book module that occurs on Drupal 10.2:
#3456958: BookOutline" plugin does not exist

megachriz’s picture

megachriz
Primary language Dutch
commented

It would be nice if drupal/book is only used when on Drupal 11. So I searched for how to conditionally composer require something and I found this:
On https://stackoverflow.com/questions/51333587/php-composer-require-depend...

So I created a metapackage called "megachriz/drupalbook" and published it on GitHub and Packagist.
https://github.com/MegaChriz/drupalbook
https://packagist.org/packages/megachriz/drupalbook

Let's see if I did this correctly.

megachriz’s picture

megachriz
Primary language Dutch
commented
Status: Active » Needs review

All tests are passing, I will check the code changes in a few days.

ankitv18’s picture

ankitv18 commented

All changes looks good and great to see all the pipelines are passing without single warning.. great improvisation.

Only thing I'm concerned is about megachriz/drupalbook as version^1 consists of only drupal/core:^10 and version^2 consists of drupal/core:^11 and drupal/book:^1.0

Rest all looks perfectly in place.

ankitv18’s picture

ankitv18 commented

With referring #5 I believe once book module fixes at bug then book issue for previous minor also get fixed and then we can get back to the drupal/book package, Correct me if I'm wrong!!

megachriz’s picture

megachriz
Primary language Dutch
commented

@ankitv18
Thanks for giving feedback!

Only thing I'm concerned is about megachriz/drupalbook as version^1 consists of only drupal/core:^10 and version^2 consists of drupal/core:^11 and drupal/book:^1.0

My idea behind this is: use drupal/book version 1 only on Drupal 11, else do not use drupal/book at all. Can you explain further what your concern is here?

With referring #5 I believe once book module fixes at bug then book issue for previous minor also get fixed and then we can get back to the drupal/book package, Correct me if I'm wrong!!

Yes, if #3456958: BookOutline" plugin does not exist gets fixed, we can stop using megachriz/drupalbook as dev dependency of Feeds and use drupal/book^1.0.1 instead.

ankitv18’s picture

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

alright then I guess this one is ready to move ahead ~~ marking this RTBC

ankitv18’s picture

ankitv18 commented

@megachriz can we merge this and plan for Drupal 11 release: https://www.drupal.org/project/feeds/issues/3430449

megachriz’s picture

megachriz
Primary language Dutch
commented

I think that August 29 would be a good date for the next release. I'd like to create releases for all Feeds related projects on the same day. The Feeds related projects that I maintain are:

  • Feeds
  • Feeds Extensible Parsers
  • Feeds Tamper
  • Tamper
  • Feeds Textarea Fetcher
  • Commerce Feeds

Commerce Feeds is not compatible with Drupal 11 yet in its dev version. That one requires #3468323: [D11] RouteNotFoundException thrown when viewing imported product variations to be fixed. I hope to finish that one and Commerce Feeds this Thursday. Then I need some time to write the release notes for each project.
Tamper and Feeds Textarea Fetcher look release ready. I did some basic testing for Feeds, Feeds Extensible Parsers and Feeds Tamper, but it would be good to test them on error handling too - because of the replacement of watchdog_exception(). And not every piece of code where watchdog_exception() was used is covered by tests. But I don't want to put too much time into testing either so the releases can still happen this month.

ankitv18’s picture

ankitv18 commented

Alrighty!! I would suggest if we merge this issue then it would unblock the next major pipeline for all the other issues.
Issue#3468323 would also gets the complete green pipeline.

  • megachriz committed 47a8421d on 8.x-3.x 
    Issue #3454788 by megachriz, ankitv18: Fixed PHPStan issues and tests...
megachriz’s picture

megachriz
Primary language Dutch
commented
Status: Reviewed & tested by the community » Fixed

Recent changes in Drupal 11 dev caused tests to fail again, but I've fixed them. 😅

Merged the code.

megachriz’s picture

megachriz
Primary language Dutch
commented

@ankitv18
Unfortunately, while working on Commerce Feeds today, I found a new bug in Feeds that gives a warning on D10 and is disruptive on D11.
Therefore, I think it's not realistic anymore to do the releases on August 29. I won't have time this weekend to fix the bug.

The bug is that when an array is used for an unique target, Drupal 11 throws an InvalidQueryException.
Some details here: https://git.drupalcode.org/project/commerce_feeds/-/merge_requests/5/dif...

I hope to create a bug report for this early next week. I'm done for today.

Status: Fixed » Closed (fixed)

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