Revealed via #1373142: Use the Testing profile. Speed up testbot by 50%

Problem

  • Renaming a node type bundle causes notices on manage fields page (and fields on the bundle are likely not moved).

Reproduce

  1. drush -y si minimal; drush -y en field_ui
  2. visit 'admin/structure/types/add', create content type
  3. edit content type from 2., just change the machine name and save
  4. visit the manage fields page for the content type from 2.
  5. FAIL, notice as in the test.

Attached patch has a test that does that, and throws the test exception.

Looks like some random dependency on something that amounts to a missing field_cache_clear() call.

Related issues

#1079966: Notice: Undefined index: [custom_content_type_machine_name] in _field_ui_bundle_admin_path(), line 309 in field_ui.module

Files: 
CommentFileSizeAuthor
#37 1375452-37-tests-only.patch2.77 KBdcam
FAILED: [[SimpleTest]]: [MySQL] 40,348 pass(es), 0 fail(s), and 1 exception(s).
[ View ]
#37 1375452-37.patch4.37 KBdcam
PASSED: [[SimpleTest]]: [MySQL] 40,360 pass(es).
[ View ]
#32 1375452-32.patch1.17 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 48,820 pass(es).
[ View ]
#30 1375452-30.patch1.09 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 48,772 pass(es).
[ View ]
#29 1375452-29.patch1.05 KBswentel
PASSED: [[SimpleTest]]: [MySQL] 48,766 pass(es).
[ View ]
#19 rename-bundle-137452-19.patch3.4 KBoriol_e9g
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch rename-bundle-137452-19.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#17 rename-bundle-only-tests-137452-17.patch1.84 KBoriol_e9g
FAILED: [[SimpleTest]]: [MySQL] 37,324 pass(es), 0 fail(s), and 1 exception(es).
[ View ]
#17 rename-bundle-137452-17.patch3.4 KBoriol_e9g
PASSED: [[SimpleTest]]: [MySQL] 37,310 pass(es).
[ View ]
#11 rename-bundle-1375452-11.patch3.4 KBbeejeebus
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch rename-bundle-1375452-11.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]
#8 drupal8.node-bundle-rename.8.patch3.53 KBsun
PASSED: [[SimpleTest]]: [MySQL] 34,246 pass(es).
[ View ]
#7 drupal8.node-bundle-rename.7.patch3.01 KBsun
FAILED: [[SimpleTest]]: [MySQL] 34,091 pass(es), 1 fail(s), and 1 exception(es).
[ View ]
#4 drupal8.field-attach-bundle-rename.4.patch1.15 KBsun
PASSED: [[SimpleTest]]: [MySQL] 34,235 pass(es).
[ View ]
node_field_ui_bug.patch1.63 KBbeejeebus
FAILED: [[SimpleTest]]: [MySQL] 34,222 pass(es), 0 fail(s), and 1 exception(es).
[ View ]

Comments

Status:Active» Needs review

Status:Needs review» Needs work

The last submitted patch, node_field_ui_bug.patch, failed testing.

Component:node system» field system
Status:Needs work» Needs review
StatusFileSize
new1.15 KB
PASSED: [[SimpleTest]]: [MySQL] 34,235 pass(es).
[ View ]

Thanks @catch for digging out that issue -- it's pretty obvious now.

Status:Needs review» Needs work

Apparently, #4 does not fix the bug, so we're back to square one.

However, clearing the entity info cache seems like a good idea either way, since bundles are also cached there, no?

this bug is caused by stale info from _node_types_build().

when we clear the entity info cache, the next call to entity_get_info calls into module's hook_entity_info implementations.

node_entity_info calls in to node_type_get_name, which returns stale info right back into our freshly rebuilt entity info. awesomeness.

Priority:Normal» Major
Status:Needs work» Needs review
StatusFileSize
new3.01 KB
FAILED: [[SimpleTest]]: [MySQL] 34,091 pass(es), 1 fail(s), and 1 exception(es).
[ View ]

First of all, attached patch exposes the bug.

StatusFileSize
new3.53 KB
PASSED: [[SimpleTest]]: [MySQL] 34,246 pass(es).
[ View ]

I guess this would be the most simple solution, err, stop-gap fix.

Status:Needs review» Reviewed & tested by the community

yes, i think we need to go with the simple fix for now, then open follow up patches or link to existing cleanups in the entity system.

this is a simple patch, with a test, i think its RTBC.

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

I've opened #1376884: Use configuration for entity types ;)

This looks good to me, comes with a nice test, and fixes a bug exposed by testing cleanup. Committed/pushed to 8.x.

Status:Patch (to be ported)» Needs review
StatusFileSize
new3.4 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch rename-bundle-1375452-11.patch. This may be a -p0 (old style) patch, which is no longer supported by the testbots.
[ View ]

and here's the D7 version.

also, look at the lists of likely related issues in #686938-61: Undefined index: admin in _field_ui_bundle_admin_path() ... ewwww.

The patch in #11 works for me, but I haven't tested the test.

Status:Needs review» Reviewed & tested by the community

Go!

#11: rename-bundle-1375452-11.patch queued for re-testing.

Status:Reviewed & tested by the community» Needs work

The last submitted patch, rename-bundle-1375452-11.patch, failed testing.

StatusFileSize
new3.4 KB
PASSED: [[SimpleTest]]: [MySQL] 37,310 pass(es).
[ View ]
new1.84 KB
FAILED: [[SimpleTest]]: [MySQL] 37,324 pass(es), 0 fail(s), and 1 exception(es).
[ View ]

Rerolled. Two patches, one with only the tests and other with all.

Status:Needs work» Needs review

StatusFileSize
new3.4 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch rename-bundle-137452-19.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

Same with small whitespaces fix.

Status:Needs review» Reviewed & tested by the community

thanks oriol_e9g, lets see what webchick thinks.

Version:7.x-dev» 8.x-dev
Status:Reviewed & tested by the community» Needs work

Confirmed that the patch clears up the bug.

The one thing I'm not as crazy about though is the test. While I can confirm that without the rest of the patch, the test fails on its own, I'm not able to reproduce it in the minimal profile by following the steps (Create a content type, a vocabulary, and a term reference field). Versus I can clearly trigger the bug with beejeebus's steps in the OP, and "rename bundle" seems like a worthy thing to have a test for.

The comments in the test also don't clear things up for me as to why this actually triggers anything because they say:

+    // Create Basic page and Article node types.
...
+    // Create a vocabulary named "Tags".

...and? :) What's the punchline? What's the condition are we checking for?

So if we could either re-work the tests to be more along the lines of beejeebus's steps in #1, *or* more clearly articulate in these tests what we're testing here (with an assertion or similar) that would really help.

There is a FieldUIManageFieldsTestCase::testRenameBundle() test already.

However, the test only reveals that the code throws an exception with the entity_info_cache_clear() from #7.

The lines outlined in #21 are technically irrelevant for this issue - they merely make FieldUIManageFieldsTestCase work with the Testing profile (the master patch revealed this bug).

That said, we could improve testRenameBundle() to test the expected results a bit more explicitly. All it currently does is:

    $this->drupalPost('admin/structure/types/manage/' . $this->hyphen_type, $edit, t('Save content type'));
    $this->drupalGet('admin/structure/types/manage/' . $hyphen_type2 . '/fields');

...without any assertions after each of those requests.

At the very least, we could assert confirmation messages, and response status codes. That said, the drupalGet() always yields a positive 200 response, because the menu router automatically falls back to admin/structure/types, in case the %node_type cannot be found - so the test should verify that the GET actually shows the Manage fields page of the renamed node type.

Title:Renaming a content type bundle causes notices on manage fields pageRenaming a content type bundle causes notices on manage fields page (test improvements)
Category:bug» task
Priority:Major» Normal
Issue tags:+needs backport to D7

This is just test fixes now, downgrading status.

Status:Needs work» Needs review
Issue tags:-needs backport to D7

#19: rename-bundle-137452-19.patch queued for re-testing.

Status:Needs review» Needs work

The last submitted patch, rename-bundle-137452-19.patch, failed testing.

Status:Needs work» Needs review

#19: rename-bundle-137452-19.patch queued for re-testing.

Status:Needs review» Needs work
Issue tags:+needs backport to D7

The last submitted patch, rename-bundle-137452-19.patch, failed testing.

Hello,

as the patch failed testing, is someone interested to modify it?

Thanks

Status:Needs work» Needs review
StatusFileSize
new1.05 KB
PASSED: [[SimpleTest]]: [MySQL] 48,766 pass(es).
[ View ]

so the test should verify that the GET actually shows the Manage fields page of the renamed node type.

This should do it.

StatusFileSize
new1.09 KB
PASSED: [[SimpleTest]]: [MySQL] 48,772 pass(es).
[ View ]

Actually, this is even better.

Works for me, but would need a phpdoc block on ManageFieldsTest::manageFieldsPage() then.

StatusFileSize
new1.17 KB
PASSED: [[SimpleTest]]: [MySQL] 48,820 pass(es).
[ View ]

Status:Needs review» Reviewed & tested by the community

Thanks, works for me.

#32: 1375452-32.patch queued for re-testing.

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

Oops, sorry. That was me sucking at Git. :P

Committed and pushed to 8.x. Thanks!

Moving to 7.x for backport.

Has this been backported? I've got the latest drupal version, 7.21, installed and I'm still getting these errors.

Status:Patch (to be ported)» Needs review
StatusFileSize
new4.37 KB
PASSED: [[SimpleTest]]: [MySQL] 40,360 pass(es).
[ View ]
new2.77 KB
FAILED: [[SimpleTest]]: [MySQL] 40,348 pass(es), 0 fail(s), and 1 exception(s).
[ View ]

Backported #8 and #32 to D7.

Issue tags:-needs backport to D7

#37: 1375452-37.patch queued for re-testing.
oops, sorry.
The patch doesn't seem to work for kickstart;
"doesn't seem" because maybe I made another error :-(

Issue tags:+needs backport to D7

#37: 1375452-37.patch queued for re-testing.

Status:Needs review» Needs work

#37 did not fix the issue for me using Drupal 7.23. I encountered the error after enabling the core Blog module and then disabling it. Patch #7 in #1898110: Notice: Undefined index: blog provides a work around for the issue.

Issue summary:View changes

Updated issue summary.