Remove all fixtures except for one: `fake_site`

Put a not in the summary about doing the kernel test first the functional test are a bit tricker because of getting the stage directory

We have 1 child issue left #3321904: Remove automatic_updates_extensions/tests/fixtures/stage_composer in favor of using StageFixtureManipulator, tagging as core-mvp because that issue is tagged. Then we can close this issue

Someone please, check how much work is left on this. If there specific tests still with fixtures please make issue for them(if there isn't one).

also if there tests where we will always need a fixture, say produce an edge error please note in the summary so we don't have to do this search again.

So for every test that still uses a fixture folder either

1. make sure it has an issue
2. list it here as to why it should still have a fixture

The two cases where we need to have fixtures.

1. package_manager/tests/src/Build/PackageUpdateTest.php => has a fixture (alpha/updated_module) required for mocking pulling from composer. **I'm Wondering what we are copying to a temp directory could be simulated but I'm not sure.**
2. package_manager/tests/src/Kernel/FixtureManipulatorTest.php => has a fixture FixtureUtilityTraitTest which is also required to prove that Manipulator works on existing fixtures

I could not find any more unused/can be removed fixture files anywhere.

1. Can we get rid of \Drupal\Tests\package_manager\Kernel\FixtureManipulatorTest::testModifyPackage()'s reliance on the existing_correct_fixture fixture by using the fake_site fixture? 🤔
2. Can we get rid of \Drupal\Tests\package_manager\Build\PackageUpdateTest::testPackageUpdate()'s reliance on the alpha and updated_module fixtures by using the manipulator to create those circumstances? 🤔

If the answer is "yes" twice, then we could end up with a drastically simpler state: a single fixture.

Assigning to @tedbow for telling me why this is not possible, but optimistically retitling because that'd make this issue actionable and no longer a meta! 🤓

1. re #13.1 I think this is worth a try. I am not sure but I think we should be able to.
2. #13.2 updated_module is special case where we have 2 version of modules with 2 different code and we test updating between the 2 and make sure call the correct version of code \Drupal\updated_module\PostApplySubscriber. The fixture manipulator isn't made to do this fact that you can inspect the 2 versions of the module I think make understanding it easier.

   for alpha we don't need manipulation of the installed.php or installed.json. We could use \Drupal\fixture_manipulator\FixtureManipulator::addProjectAtPath but the 2 versions need a composer.json with different version values which the FixtureManipulator doesn't handle. so I would say these can't be replaced either.

   Maybe we should move these modules under a new folder package_manager/tests/fixtures/build_test_projects or something we could but a README.md explain how this fixtures are used because they are different than all the other fixtures.

So here is what I think we should do

1. Create a new issue to remove the need for automatic_updates_extensions/tests/fixtures/fake-site this should be tagged contrib-only and does not need the sprint, not high priority
2. In this issue remove see if we can remove package_manager/tests/fixtures/FixtureUtilityTraitTest/existing_correct_fixture

   move updated_module, alpha into a sub-folder for build test related repos(this could be done in different follow-up)

Unassigning so anyone could take this on

So from what i understand under So here is what I think we should do 1 is not core-mvp and 2 is what's not clear is it core-mvp or not ?

re #15

Yes #14.1 is not core-mvp. We will need a new issue for that and tag it as contrib-only because it deals with automatic_updates_extensions
#14.2 is core-mvp and should be done in this issue.

This is ready AFAICT, and the documentation helps a lot. But I think the documentation could be far clearer still.

I left 4 suggestions: 2 of which are trivial Markdown tweaks, the other 2 are providing more context to the documentation that was already being added), and one also contains a question that may require the creation of a follow-up issue.

Assigning @tedbow for accepting and merging my suggestions, but this is definitely in the RTBC realm :)

Merged because the last change from gitlab was just to readme. So don't need to wait on tests 🤞🏻(will check 8.x-2.x)

thanks!

You did not respond to:

⇒ this actually makes me realize that BuildTests should never use (Active|Stage)FixtureManipulator? 🤔 Because doing so means we're no longer testing php-tuf/composer-stager

Thoughts, @tedbow?

→ that is what could merit a follow-up 😊

@Wim Leers sorry. Yes I agree with the comment that (Active|Stage)FixtureManipulator should not be used in build tests.

The only case I could see were we might use it is if used `\Drupal\fixture_manipulator\FixtureManipulator::addProjectAtPath` to the active directory to simulate having a project that is not known to composer. But we haven't had a need for that yet.

that is what could merit a follow-up 😊

I follow up to make the comment more specific or to somehow forbid this

AFAICT we not using (Active|Stage)FixtureManipulator in build tests right now. Did you find a case where we are?

Did you find a case where we are?

No, I did not. But we were contemplating it in this very issue — that is literally what the docs we just committed (and you wrote the key parts of it in #14) are saying: "we can't use the manipulator for reasons X and Y" — which wouldn't even be a consideration if build tests could not use the manipulators!

I created #3338392: Document that StageFixtureManipulator should not be used in build tests as a core-post-mvp lets move discussion there and close this