Problem/Motivation

I have found the relation between \Drupal\fixture_manipulator\StageFixtureManipulator and \Drupal\package_manager_bypass\Beginner was always very hard for me to understand, especially how the logic around object destruction works and ensuring that `commitChanges()` gets called because the `StageFixtureManipulator` are serialized in state between requests.

@wim leers did some amazing work to make the current logic work but there seems to be no way to make the current system simple and we are still finding edge cases.

Because of this understanding how this works you have to read many places in our code base and I still find it challenging to understand.

Proposed resolution

Remove the need to have StageFixtureManipulator objects serialized in state at all by adding methods to Beginner that just store package versions in state, not objects.

Remaining tasks

Issue fork automatic_updates-3336255

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

tedbow created an issue. See original summary.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: Unassigned » tedbow

working on an idea

tedbow opened merge request !672

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Issue summary: View changes
Related issues: +#3325654: Improve the user experience of having your staged update deleted before it was applied, +#3322913: Create an easy way for functional tests to simulate an update (and update kernel tests to use the same)

updating summary

relating #3325654: Improve the user experience of having your staged update deleted before it was applied because in the summary I explain we just found an edge case there.

tedbow opened merge request !673

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented

Thought of what I think is an even simpler solution. Working on it in 3336255-subscriber-manipulator

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » Unassigned
Priority: Minor » Normal
Status: Active » Needs review

Ok I like 3336255-subscriber-manipulator much better.

I still want to self review the new MR but ready for review.

tedbow closed merge request !672

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Related issues: +#3327391: Improve FixtureManipulator DX: validate package name + ensure StageFixtureManipulator is committed + ensure `package_manager_bypass_composer_stager` is not set to FALSE

Went through this in detail, had a few questions, but man … this is SO MUCH BETTER! :D

Much simpler than #3327391: Improve FixtureManipulator DX: validate package name + ensure StageFixtureManipulator is committed + ensure `package_manager_bypass_composer_stager` is not set to FALSE 🤩

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: Unassigned » wim leers
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » tedbow
Status: Needs review » Needs work 
Testing Drupal\Tests\automatic_updates\Kernel\CronUpdaterTest
.......E.......................                                   31 / 31 (100%)

Time: 00:54.511, Memory: 6.00 MB

There was 1 error:

1) Drupal\Tests\automatic_updates\Kernel\CronUpdaterTest::testStageDestroyedOnError with data set "post-create exception" ('Drupal\package_manager\Event\...eEvent', 'Exception')
LogicException: The StageFixtureManipulator has arguments that were not cleared. This likely means that the PostCreateEvent was never fired.
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Assigned: tedbow » wim leers
Status: Needs work » Needs review

tedbow opened merge request !675

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Assigned: wim leers » Unassigned
Status: Needs review » Reviewed & tested by the community

Per https://git.drupalcode.org/project/automatic_updates/-/merge_requests/67..., RTBC'ing merge request 675, which is a nice simplification over merge request 673! 👏

  • tedbow committed 1a75de35 on 8.x-2.x 
    Issue #3336255 by tedbow, Wim Leers: Explore simplifying our fixture...
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Reviewed & tested by the community » Fixed

Thanks for the feedback @Wim Leers!

Status: Fixed » Closed (fixed)

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