Add test coverage for tablesort header link titles

+++ b/core/modules/system/src/Tests/Database/SelectTableSortDefaultTest.php
@@ -81,4 +81,22 @@ function testTableSortDefaultSort() {
+    // Verify that the links in the header row are correct.
+    // 1. Look for a link with the expected text.
+    // 2. Make sure the href is the page we just requested.
+    // 3. Make sure the link title is there, and correct.
+

These comments are unnecessary as a block. Either remove them or put each respective one above the actual check.

I think adding test coverage should be a task per https://www.drupal.org/core/issue-category.

Restest passed. Analysis of test code made sense.

+++ b/core/modules/system/src/Tests/Database/SelectTableSortDefaultTest.php
@@ -81,4 +81,17 @@ function testTableSortDefaultSort() {
+  function testTableSortHeaderLinks() {

Let's merge this into testTableSortDefaultSort since having two test methods do assertions against the same page seems wasteful.

I've completed the beta evaluation properly.

+++ b/core/modules/system/src/Tests/Database/SelectTableSortDefaultTest.php
@@ -71,7 +71,8 @@ function testTableSortQueryFirst() {
+   * Confirms that no error is thrown if no sort is set in a tableselect, and
+   * that header links are correct.
    */

The first line must be one sentence, alone.

Tweaked the comments.

Here at drupalcon LA sprints. I'll take a look at this.

Matches drupal code standards. https://www.drupal.org/coding-standards

I applied the patch, seems OK. I ran the database tests locally, and they passed.

Here at the LA sprints. Going to remove some extra white-space from the previous patch and see if I can break the code, this is to see if the test actually works.

Thanks for working on this issue. With this patch we can make the test coverage of Drupal 8 again better! I did still find one nitpick from the patch:

+++ b/core/modules/system/src/Tests/Database/SelectTableSortDefaultTest.php
@@ -71,7 +71,10 @@ function testTableSortQueryFirst() {
+   *  Specifically that no sort is set in a tableselect, and that header links
+   *  are correct.

Indentation is wrong

thank you fixed.

I see martin107 already submitted the patch for the indentation but I broke code in-order to be sure the test fails in a local dev and it did fail.
So the test seems to be working as intended.

This looks good to me.

Thanks everyone for your work here.

Whenever a patch fails tests like that, please always document what the fail was before clicking retest. Seeing two fails and retests in a row like that raises a red flag that the patch might be introducing a random test failure. Especially in a patch like this that is specifically expanding test coverage.

There are no typical gotchas like random value generation, but I'm currently running the specific test from the patch locally 100 times to ensure that there's not some random failure with the assertion.

+++ b/core/modules/system/src/Tests/Database/SelectTableSortDefaultTest.php
@@ -80,5 +83,10 @@ function testTableSortDefaultSort() {
+    $this->assertPattern('/\<a.*title\=\"' . t('sort by Username') . '\".*\>/');

An xpath assertion might be better here? There are a handful of uses of assertPattern() in core for markup, though, so I can't think of a reason to actually block the patch on that. I wondered if this could introduce a random fail in circumstances when something with the markup output changed, but also couldn't think of a way that could happen.

Leaving at RTBC while my laptop warms the house. :P

Alrighty, no fails, so I'm at least confident this isn't introducing some high-frequency random fail. :)

This issue only changes test code, so per https://www.drupal.org/core/beta-changes, this can be completed any time during the Drupal 8 beta phase. Committed and pushed to 8.0.x. Thanks!