- Issue created by @quietone
- First commit to issue fork.
- Merge request !11619Resolve #3515189 "Migratecontrollertest requires root" β (Closed) created by vensires
- π¬π·Greece vensires
I closed the MR so that anyone willing to tackle this issue may start the process from the very beginning.
To be honest though, I did try to find the comment described in the issue summary but couldn't find it at all in the code. Maybe something is off here or was it just me? - π¨π¦Canada jodavidson
Working on this at Mentored Contribution at Atlanta 2025
- πΊπΈUnited States alison
Working on this at Mentored Contribution at Atlanta 2025
- Merge request !11645Removed todo line from MigrateControllerTest β (Open) created by Unnamed author
- πΊπΈUnited States artis
Reviewed at DrupalCon Atlanta 2025. This looks like it's ready to go.
- πΊπΈUnited States mradcliffe USA
I am a little confused. The @todo comment mentions to remove the usage of the super user, but it looks like this is still being used in the test. Does the variable also need to be removed?
For those working on this issue, can you explain why this wasn't removed too?
- π¨π¦Canada jodavidson
@mradclffe. As I read the issue is was to remove the todo comment from the test as the test required superuser access.
- πΊπΈUnited States smustgrave
So what needs to be removed is that variable. Which will break the test
What needs to happen is in the setUp() the user needs to be created with necessary permissions
You can use https://www.drupal.org/project/drupal/issues/3437620 π [Meta] Fix all tests that rely on UID1's super user behavior Active as good references
- πΊπΈUnited States smustgrave
Actually I may be wrong. Will take a deeper look
- π¨π¦Canada jodavidson
Per the issue: "migrations through the UI must be run as the root user." Has that changed? Nothing in this issue mentioned changing the test. The issue was the presence of the todo, not the test itself.
I could have a go at that, but if the test __has__ to be run as superuser I think a change to another user would break the test.
- πΊπΈUnited States smustgrave
Compromise lets add a small comment as to why are keeping
If you are another contributor eager to jump in, please allow the previous poster(s) at least 48 hours to respond to feedback first, so they have the opportunity to finish what they started!
- π³πΏNew Zealand quietone
There is an existing comment in the test explaining that migrations run through the UI must must as user #1, See https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/migra....
Migrate UI tests were also discussed in π Fix Migrate Drupal UI tests that rely on UID1's super user behavior Closed: won't fix , but this one was missed.
- π³πΏNew Zealand quietone
I didn't mean to set to RTBC.
I see the problem. When I created the issue I didn't include all that lines that are to be remove. I have updated the issue summary. Sorry about that.