Posted by xjm on November 22, 2012 at 5:50am
6 followers
| Project: | Drupal core |
| Version: | 8.x-dev |
| Component: | node.module |
| Category: | task |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed (fixed) |
| Issue tags: | Testing system, VDC |
Issue Summary
SummaryLengthTest just tests the character count for rendering one node teaser, so instead of logging in a user, rendering the whole node teaser listing, etc., the test can just render the teaser with the API to check for the correct length. This makes things easier for #1806334: Convert the node listing at /node to Views and should also reduce the overhead of running SummaryLengthTest slightly.
| Attachment | Size | Status | Test result | Operations |
|---|---|---|---|---|
| summary-test.patch | 1.64 KB | Idle | PASSED: [[SimpleTest]]: [MySQL] 48,554 pass(es). | View details |
Comments
#1
The patch is looking great in general.
+++ b/core/modules/node/lib/Drupal/node/Tests/SummaryLengthTest.phpundefined@@ -46,8 +47,12 @@ function testSummaryLength() {
+ // Attempt to access the front page again and check if the summary is now
+ // only 200 characters in length.
Adapted the comment, as we don't render it directly anymore.
+++ b/core/modules/node/lib/Drupal/node/Tests/SummaryLengthTest.phpundefined@@ -46,8 +47,12 @@ function testSummaryLength() {
+ $this->drupalSetContent(drupal_render($content));
That's a great idea!
#2
Thanks @dawehner! (This is what happens when I roll patches after midnight!)
#3
The comments and assertion messages were also a bit misleading, so attached fixes that.
#4
Oh this much better explains what's going on.
+++ b/core/modules/node/lib/Drupal/node/Tests/SummaryLengthTest.phpundefined@@ -33,21 +33,28 @@ function testSummaryLength() {
+ // Render the node as a teaser again and check that the summary is now only
+ // 200 characters in length and so does not include 'What is a Drupalism?'.
Couldn't we check that the actual length is not longer then 200 chars as well?
#5
The last submitted patch, node-1847554-3.patch, failed testing.
#6
Ah yeah, might as well since we already have the render array.
#7
Oh, but actually, it will not be exactly 200 or 600 characters... just less than those, based on the trim algorithm. So I think this is okay as is. Ish.
#8
The test failure is a bot issue; it's in the process of retesting already.
#9
Is there a test to check that the teaser is not longer than X characters? Presumably there is and that would catch that the teaser is no more than 200 characters. If such a check of functionality does not exist, it should be added in the correct place.
#10
@Lars Toomre, did you read the patch or the test it modifies? That is what the test does implicitly, because the specific string that it's checking for is more than 200 characters into the summary.
Anyway, this patch is not intended to add test coverage for this feature, just to decouple the test from the node frontpage.
#11
This might be a bit less fragile.
#12
The last submitted patch, node-1847554-11.patch, failed testing.
#13
Bot issue.
#14
Committed and pushed to 8.x. Thanks!
#15
Automatically closed -- issue fixed for 2 weeks with no activity.