Refactor RecipeCommandTest::testExceptionOnRollback() so that it doesn't rely on buggy core behavior

Created on 25 April 2024, 7 months ago
Updated 1 May 2024, 7 months ago

Problem/Motivation

This is a follow-up from #3427118: If config rollback fails validation, it's impossible to tell what caused the rollback in the first place .

In that issue, we addressed the fact that errors which arise during a recipe rollback (triggered by a config validation error) "steamroll" the validation error. If the config importer doesn't succeed at validating the import it's about to do, you find out why that validation failed...but you don't get to find out why the rollback was triggered to begin with.

In that issue (and the test it added), we're taking advantage of an existing bug in core where filter usage is improperly validated during a config import. When that bug is fixed, our test will break. This issue is for refactoring the test so that it's not reliant on an upstream bug in core. :)

Proposed resolution

@alexpott proposed in Slack:

we can convert the test to use the config_import_test module and make the module throw an exception… and we can test outputting the error log from the config importer…

Sounds like a reasonable plan to me.

📌 Task
Status

Active

Version

11.0

Component

Code

Created by

🇺🇸United States phenaproxima Massachusetts

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

  • Issue created by @phenaproxima
  • 🇺🇸United States phenaproxima Massachusetts
  • Status changed to Active 7 months ago
  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    AFAICT #3427118: If config rollback fails validation, it's impossible to tell what caused the rollback in the first place didn't fix anything, and just is asserting that the same unhelpful error message that occurs during the rollback is actually printed to the CLI?

    Isn't the root cause that \Drupal\Core\EventSubscriber\ConfigImportSubscriber::validateModules() observes that the rollback changes core.extension and then "helpfully" observes that core.extension cannot be modified until the filter.format.plain_text text format stops using the media_embed filter, which core/tests/fixtures/recipes/config_rollback_exception/recipe.yml just added to that text format?

    Doesn't that then mean that we:

    1. either need to make ConfigImportSubscriber::validateModules() smarter
    2. or perform the rollback operation in two stages: first revert everything except for core.extension changes, and THEN perform the core.extension rollback, which would prevent triggering this exception in ConfigImportSubscriber::validateModules()?
  • 🇺🇸United States phenaproxima Massachusetts

    Yes - it means things in core need to change.

    But that's not in scope of the recipe system. (I had raised this same concern with @alexpott.)

  • 🇧🇪Belgium wim leers Ghent 🇧🇪🇪🇺

    #4: not necessarily — the second option in #3 requires no core changes?

  • 🇬🇧United Kingdom alexpott 🇪🇺🌍

    Well it is a bug that you can not uninstall filter format providing modules in one step via the config importer. That's nothing to do with recipes and this situation can happen outside of recipes.

Production build 0.71.5 2024