Coming from #2499971: Tests are missing @coversDefaultClass annotations.

If a module defines tests that derive from UnitTestCase, if its tests do not follow some specific PHPDoc format (which has yet to be documented #2325985: No documentation about @group @coversDefaultClass @covers), the entire page at admin/config/testing blows up with a Drupal\simpletest\Exception\MissingSummaryLineException, even if that module is not enabled.

This behaviour was changed in #2422019: Don't use reflection for parsing test annotations.

From the issue summary there, I can understand why @group is required, because that's what we're basing our discovery on. Why must there be a summary/description though?

Can we just default this to an empty string instead, and kill MissingSummaryLineException?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

larowlan’s picture

Could we fallback to the class name?

cilefen’s picture

Assigned: Unassigned » cilefen
cilefen’s picture

Assigned: cilefen » Unassigned
Status: Active » Needs review
FileSize
1.08 KB
3.11 KB

This uses the former exception message.

cilefen’s picture

Issue summary: View changes
FileSize
226.33 KB

I prefer this because it tells the developer "you didn't do something we expect should be done". But maybe a blank space would be obvious.

The last submitted patch, 3: killing_the_entire-2500031-3-TESTS.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 3: killing_the_entire-2500031-3.patch, failed testing.

cilefen’s picture

Status: Needs work » Needs review
webchick’s picture

It's funny because I had to stare at that screenshot for multiple seconds to understand what you meant. :) I think a blank space in this case is actually better, because that would definitely stand out against the other noise in the table.

cilefen’s picture

cilefen’s picture

Status: Needs review » Needs work

Oops - wrong patch, correct interdiff though.

cilefen’s picture

Status: Needs work » Needs review
FileSize
3.06 KB
webchick’s picture

Assigned: Unassigned » alexpott
Status: Needs review » Reviewed & tested by the community

Nice! That works for me. Sending over to alexpott though, since it was his patch that went in.

alexpott’s picture

Status: Reviewed & tested by the community » Fixed

I can't remember why I coded it so strict... this change makes sense.

This is test only code and therefore permitted during the beta. Committed fbeaa7e and pushed to 8.0.x. Thanks!

  • alexpott committed fbeaa7e on 8.0.x
    Issue #2500031 by cilefen: Killing the entire Testing list page if any...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.