Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.Follow-up from #2648284: Service container rebuild is required when events are configured:
oh, I just figured that we are doing this at the wrong place. The form is not the right place as this will miss any rules that get synced via CMI and/or are created via API. But moving this to a config entity crud event should be fine - once done so we should be able to remove the manual kernel rebuilds from EventIntegrationTest also.
Comments
Comment #2
fagoComment #3
vasi1186 CreditAttribution: vasi1186 at Amazee Labs commentedComment #4
vasi1186 CreditAttribution: vasi1186 at Amazee Labs commentedHere is the pull request: https://github.com/fago/rules/pull/381
One thing maybe: should we create more tests that check more extensive the rules update/delete and the container rebuild, or do you think the current ones are enough?
Comment #5
vasi1186 CreditAttribution: vasi1186 at Amazee Labs commentedtravis reported some issues.
Comment #6
klausiThe usual random test fail we see sometimes.
Comment #7
fagoThanks, changes look really good. I think test coverage is ok as we've the functional API test coverage that does not need the manual rebuilds any more now. IT would be nice to cover the deletion case as well, but I don't think that would be easy to test?
Thus, I think that's good enough for now. Thanks, merged!
Comment #9
fagoFYI: The tests issue: #2659954: TempStorageTest::testLocking has random fails
Comment #10
vasi1186 CreditAttribution: vasi1186 at Amazee Labs commentedYes, testing the deletion is a bit harder to automate. Basically, we would have to check the container and see that the generic event is not registered. I guess the code in the test would be something similar with what we had before int he
isRuleEventRegistered()method here https://github.com/fago/rules/pull/364/filesI don't know how crucial is to have this tests automated, I personally see this as a minor issue, because it does not affect the behavior of the site, and the container will be anyway rebuilt during the next cache clear or the next time when the user creates a rule.