Problem/Motivation

Remove the active composer metadata fixture files in\Drupal\Tests\automatic_updates_extensions\Functional\UpdaterFormTest

Steps to reproduce

Proposed resolution

Remaining tasks

Create a follow-up to remove the staged composer fixture files. This is a bit more tricky than kernel tests so it should be its own issue

User interface changes

API changes

Data model changes

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:

Comments

kunal.sachdev created an issue. See original summary.

kunal.sachdev’s picture

Assigned: Unassigned » kunal.sachdev

tedbow’s picture

worked with @kunal.sachdev on this. Functional tests are more complicated because you have to get the stage directory after the update has been stages so you can use \Drupal\Tests\package_manager\Traits\FixtureUtilityTrait::modifyPackage on the stage directory.

Since we don't know the stage id we can't claim the stage to get the directory.

We will probably need a helper function like getSTageDirectory for functional test or we could make a wrapper around modifyPackage like modifyStagedPackage that would take care of this for us

wim leers’s picture

Title: Make UpdaterFormTest fixture less » Make UpdaterFormTest fixture-less
kunal.sachdev’s picture

I have some test failures locally with error :

Exception : Warning: Undefined array key "type"
Drupal\package_manager\ComposerUtility->getProjectForPackage()() (Line: 240)
wim leers’s picture

Status: Active » Needs work

#6:

  1. What is the stack trace for it?
  2. What are the contents of the array?
tedbow’s picture

Status: Needs work » Postponed
Related issues: +#3320487: fake-site fixtures has invalid packages in composer files

this is a related problem we have to fix first #3320487: fake-site fixtures has invalid packages in composer files

wim leers’s picture

Title: Make UpdaterFormTest fixture-less » [PP-1] Make UpdaterFormTest fixture-less
kunal.sachdev’s picture

Title: [PP-1] Make UpdaterFormTest fixture-less » Make UpdaterFormTest fixture-less
Status: Postponed » Needs work

Blocker is in

kunal.sachdev’s picture

Status: Needs work » Needs review
wim leers’s picture

1 review, needs input from @tedbow.

wim leers’s picture

Assigned: kunal.sachdev » tedbow
tedbow’s picture

Title: Make UpdaterFormTest fixture-less » Remove active composer fixture files in UpdaterFormTest
Issue summary: View changes
Issue tags: +Needs follow-up
tedbow’s picture

Assigned: tedbow » kunal.sachdev
Status: Needs review » Needs work

For MR comments,

tedbow’s picture

Issue tags: +sprint
kunal.sachdev’s picture

Status: Needs work » Needs review
wim leers’s picture

Assigned: kunal.sachdev » tedbow

Green and the last 3 commits look clean … I can't review this quite yet, so assigning to @tedbow 😊

tedbow’s picture

Assigned: tedbow » kunal.sachdev
Status: Needs review » Needs work

@kunal.sachdev getting close. thanks

Also we still have a follow-up issue that we need to make from the summary, I think. If it already exists lets add it as a related issue and remove the "Needs follow-up" tag

kunal.sachdev’s picture

kunal.sachdev’s picture

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

Status: Needs review » Reviewed & tested by the community

@kunal.sachdev thanks for creating #3321904: Remove automatic_updates_extensions/tests/fixtures/stage_composer in favor of using StageFixtureManipulator

I think this looks good. I am glad the test changes are pretty small but it still lets us get rid of a bunch of fixture files. I think this speak the value of the `addPackage`, `modifyPackage` and `removePackage` methods 🎉.

Once we do #3321236: Add actual project folders in \Drupal\Tests\package_manager\Traits\FixtureUtilityTrait::addPackage I think we would also be able remove the fixture folders such as automatic_updates_extensions/tests/fixtures/two_projects/web/projects/aaa_automatic_updates_test

wim leers’s picture

Indeed, +31 -161 is a nice diff stat 👏

  • tedbow committed 58fd3ff on 8.x-2.x authored by kunal.sachdev
    Issue #3318625 by kunal.sachdev, tedbow, Wim Leers: Remove active...
tedbow’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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

tedbow’s picture

Issue tags: +core-mvp