- 🇳🇿New Zealand quietone
This was a bug smash triage issue today.
We still need some tests here.
- First commit to issue fork.
- 🇺🇸United States dcam
I don't think the test changes are any good. Neither undoing the fix nor removing
d7_menufrom the test's list of migrations will make the test fail. So I'm leaving the Needs Tests tag in place. But I'm marking this as Needs Review in hope that a subsystem maintainer will advise on what might be wrong or an alternate test strategy. - 🇫🇷France andypost
btw menus probably should be required migration as core has fixed default list independently from menu module since 8.x #2085243: Rename Menu module into Menu UI module →
- 🇺🇸United States mikelutz Michigan, USA
I don't think the test changes are any good. Neither undoing the fix nor removing d7_menu from the test's list of migrations will make the test fail. So I'm leaving the Needs Tests tag in place. But I'm marking this as Needs Review in hope that a subsystem maintainer will advise on what might be wrong or an alternate test strategy.
You can't make a test for this based on looking at the system after the migrations have run, because the migration and the system do not care what order they are run. If the block runs first, it's saved with an invalid plugin for a hot minute, and then when the menu migration runs, the plugin is valid and everything is fine. So you can't do a test based on the final system state; the only way to test is to inject a mock logger or check the logs to make sure the warning is never logged.
