TermTest.php testing false positive [#2021075]

Breaking issue out from #1914694: Errors appearing in testbot Error logs..

This code from TermTest.php:

<?php
    // Check that the term edit page does not try to interpret additional path
    // components as arguments for taxonomy_term_form().
    $this->drupalGet('taxonomy/term/' . $term->tid . '/edit/' . $this->randomName());
?>

Causes the following in testbot logs:

Uncaught PHP Exception InvalidArgumentException: "Missing 'form: eixEYdYX' for entity 'taxonomy_term'" at /var/lib/drupaltestbot/sites/default/files/checkout/core/lib/Drupal/Core/Entity/EntityManager.php line 115

As Berdir described at https://drupal.org/node/1914694#comment-7055418:

This results in that additional argument being passed to entity_get_form() and the exception. We need to fix that and add an assertion to actually register that it does in fact not work. Although it will stop working with the new router system.

Comment	File	Size	Author
#1	2021075.taxonomy-term-edit-menu-item-arguments.patch	1.66 KB	rszrama
#1

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

rszrama CreditAttribution: rszrama commented 26 June 2013 at 21:26

Priority:	Major	» Normal
Status:	Active	» Needs review

File	Size
2021075.taxonomy-term-edit-menu-item-arguments.patch	1.66 KB

There were several old issues here, as the page callback is no longer taxonomy_term_form() as a couple of comments indicated, and the comments in taxonomy_menu() were incorrect anyways - they weren't passing NULL as the second argument. I've updated the menu item to pass the expected default arguments to entity_get_form() and added an assertion to TermTest.php. The old request was triggering a 500 response due to the fatal error, so my assertion simply checks to ensure we get back a 200, indicating the extraneous path argument is not being passed to the page callback.

Correct me if I'm wrong, but I don't see how this is considered a major bug. Is any problem with tests automatically assumed to be majority or higher?

Comment #2

jthorson CreditAttribution: jthorson commented 27 June 2013 at 02:19

I went with major for the combined effect that this and about 4 other uncaught exceptions have on the testing infrastructure, as they fill the apache error logs with noise and make it difficult to isolate test-specific issues ... so while not a big deal in isolation, it is a contributor to a larger problem.

I've also received mixed messages on issues like this, with suggestions that broken tests should all be critical by default, while others I've opened as critical were bumped down to major

In any case, thanks ... the more of these we can remove from the testbot logs, the happier I'll be! :D

Comment #3

jthorson CreditAttribution: jthorson commented 27 June 2013 at 03:27

Status:

Needs review

» Reviewed & tested by the community

While I didn't go deep into the test logic, I can confirm that this does indeed solve the issue of the uncaught exception in the testbot logs ... no more "missing form for entity 'taxonomy_term'" messages appearing in the error log with this patch in place.