Problem/Motivation

Discovered in #3316368-44: Remove our runtime dependency on composer/composer: remove ComposerUtility.

See:

  • getData()
  • assertOriginalFixturePackagesUnchanged()
  • testAddPackage()
  • testModifyPackageConfig()
  • …

These rely on parsing composer/vendor/installed.json, which is not public Composer API, but something internal.

Proposed resolution

That should be able to use ComposerInspector now — so we can stop relying on composer/vendor/installed.json 😊

Or … in quite a few of these cases, it could be argued that the test coverage was only necessary when we were still using ComposerUtility, which #3316368: Remove our runtime dependency on composer/composer: remove ComposerUtility removed… ⚠️

⇒ this can only be worked on by @phenaproxima or @tedbow

Remaining tasks

User interface changes

None.

API changes

None.

Data model changes

None.

Issue fork automatic_updates-3347165

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

Wim Leers created an issue. See original summary.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Parent issue: #3347164: Use Composer commands instead of parsing installed.json in TemplateProjectTestBase »
Related issues: +#3316368: Remove our runtime dependency on composer/composer: remove ComposerUtility
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Title: [PP-1] Remove FixtureManipulatorTest's reliance on installed.json & installed.php — use ComposerInspector instead? » Remove FixtureManipulatorTest's reliance on installed.json & installed.php — use ComposerInspector instead?
Status: Postponed » Active

Unblocked!

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Assigned: Unassigned » tedbow

⇒ this can only be worked on by @phenaproxima or @tedbow

I was not involved with creating FixtureManipulator, so I think this falls squarely within @tedbow's wheelhouse.

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

tedbow opened merge request !782

tedbow’s picture

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

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +beta target
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented

A couple of minor things but this makes sense overall, +1 RTBC.

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

Solid remarks :)

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Status: Reviewed & tested by the community » Needs work

Looks like there are merge conflicts. I'm not sure how to resolve them; reassigning to @tedbow for that.

kunal.sachdev’s picture

kunal.sachdev commented
Assigned: Unassigned » kunal.sachdev
kunal.sachdev’s picture

kunal.sachdev commented
Status: Needs work » Needs review

I tried to resolve the merge conflicts but not sure if it is correct.

kunal.sachdev’s picture

kunal.sachdev commented
Assigned: kunal.sachdev » Unassigned
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Assigned: Unassigned » kunal.sachdev
Status: Needs review » Needs work

I think this looks really good. Few very tiny things, but after that I think this can go to @tedbow for final review.

kunal.sachdev’s picture

kunal.sachdev commented
Assigned: kunal.sachdev » tedbow
Status: Needs work » Needs review
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Issue tags: +sprint
tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Status: Needs review » Reviewed & tested by the community
phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Title: Remove FixtureManipulatorTest's reliance on installed.json & installed.php — use ComposerInspector instead? » Remove FixtureManipulatorTest's reliance on installed.json and installed.php

phenaproxima’s picture

phenaproxima
he/him
Primary language English
Location Massachusetts
commented
Assigned: tedbow » Unassigned
Status: Reviewed & tested by the community » Fixed

Finally!

wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented

So much simpler! 🤩

Status: Fixed » Closed (fixed)

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