Skip to content
Snippets Groups Projects

Issue #3246203: Replace PreCommitEvent with PreApplyEvent

Merged Issue #3246203: Replace PreCommitEvent with PreApplyEvent
issue/automatic_updates-3246203:3246203-replace-precommitevent-with into 8.x-2.x
1 unresolved thread
Merged Adam G-H requested to merge issue/automatic_updates-3246203:3246203-replace-precommitevent-with into 8.x-2.x
1 unresolved thread

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • Ted Bowman
    Ted Bowman @tedbow started a thread on the diff
    • tests/src/Unit/StagedProjectsValidatorTest.php → tests/src/Kernel/ReadinessValidation/StagedProjectsValidatorTest.php
    1 1 <?php
    2 2
    3 namespace Drupal\Tests\automatic_updates\Unit;
    3 namespace Drupal\Tests\automatic_updates\Kernel\ReadinessValidation;
    • Ted Bowman
      Ted Bowman @tedbow ·
      Maintainer

      I think we were using this namespace for tests that are "Readiness" checks. This validator deals with after an update is staged

    • Ted Bowman
      Ted Bowman @tedbow ·
      Maintainer

      I think all other validators tested in this namespace at least listen to \Drupal\automatic_updates\Event\ReadinessCheckEvent though some also listen to other events. Maybe we should actually change this namespace to just \Validation

      Another thing this brings up is that for validators such as PendingUpdatesValidator that subscribe to ReadinessCheckEvent and PreStartEvent the fact that we use assertCheckerResultsFromManager() to test that it listens to ReadinessCheckEvent but not PreStartEvent if it is important that it listens to PreStartEvent we should also add test coverage for this too. I don't think it would be that are hard. obviously another issue.

      Also this points out that most of validators that listen to ReadinessCheckEvent should also listen to PreStartEvent because the update maybe started from the API and not just the form. For instance DiskSpaceValidator and ComposerExecutableValidator should be able both stop an update trigger via an API call but now they can't

      I will make follow ups. Not sure this needs to be done before we finish refactoring logic into package_manager.

    • Please register or sign in to reply
  • Ted Bowman
  • Adam G-H added 1 commit

    added 1 commit

    Compare with previous version

  • Adam G-H merged

    merged

    • Please register or sign in to reply