Fatal error when sorting dblog by type

Fix looks good.

Sounds like this needs a test.

Test and fix.

#4: fix-test-only.patch queued for re-testing.

Re-roll

+++ b/core/modules/dblog/lib/Drupal/dblog/Tests/DBLogTest.phpundefined
@@ -518,6 +518,10 @@ protected function testFilter() {
+    $this->drupalGet('admin/reports/dblog', array('query' => array('order' => 'Type')));
+    $this->assertResponse(200);
+    $this->assertText('Operations', 'Order by worked');

Operations should actually go through t().

I'm not sure if the assertion message here is necessary. They sometimes aren't that useful because they hide the string that we are actually looking for.

So locally I was getting a 200 for the inital test but the bot gets a 500 so this should work.

Hm, might be a misunderstanding :)

I actually liked having the assertText() there because, as you figured out, the http response code depends on the php configuration. What I was talking about is just the assertion message, the "'Order by worked'" which essentially hides that we are looking for the Operations string. So you have to look at the code to understand what the assertion does.

Ah my bad, take 2!

Starting to feel bad :)

The assertion text message is optional. And if you leave it out, then the default message is almost exactly what you did there: "'"@text" found'". So all you need is assertText(t('Operations')) and then it's good to go :)

Ok but that is not the approached used elsewhere in the test in question eg

$this->assertText(t('Database log cleared.'), 'Confirmation message found');

And also through out verifyReports() the optional string is always present. So I can change the patch but the tests for dblog look like they need a follow up if we are going down this approach because we lose consistency.

Lets keep it consistent me thinks

Yeah, let's get a minor follow-up for the assertion messages clean-up. For now, I agree with keeping things consistent.

Committed and pushed to 8.x. Thanks!