Problem/Motivation

#1328014: PDOException when saving a node on non-English languages Introduced a really long string of Chinese characters into the DbLogTest class.

There was even a ticket requesting to document this change: #2671344: Better documentation for suspicious encoded string in dblog tests

Also, there is no proof this fixed the issue. There is no failing testcase (because on that time we didn't have that practice).

And finally this makes #2848914: Move DbLogTest::generateLogEntries() into a Trait quite complicated to review.

Proposed resolution

  • Move the suspicious string into a KernelTest to ensure this is working as expected.
  • Remove the encoded string from DbLogTest to make easier to read this file.

Remaining tasks

User interface changes

None

API changes

None

CommentFileSizeAuthor
#8 interdiff-2882525-5-8.txt787 bytesdagmar
#8 2882525-8.patch6.25 KBdagmar
#5 interdiff-2882525-2-5.txt979 bytesmitrpaka
#5 2882525-5.patch5.99 KBmitrpaka
#2 2882525-2.patch7.2 KBdagmar

Comments

dagmar created an issue. See original summary.

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented
Title: Remove suspicious string in DblogTest into a KernelTest » Move suspicious string in DblogTest into a KernelTest
Status: Active » Needs review
StatusFileSize
new 2882525-2.patch7.2 KB

Here is the failing test case before the fix introduced in #1328014: PDOException when saving a node on non-English languages.

Status: Needs review » Needs work

The last submitted patch, 2: 2882525-2.patch, failed testing.

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented
Issue tags: +Novice

Now that this exposed the bug. Remove the changes introduced in the install file is a novice task.

mitrpaka’s picture

mitrpaka commented
Status: Needs work » Needs review
StatusFileSize
new 2882525-5.patch5.99 KB
new interdiff-2882525-2-5.txt979 bytes

Updated patch as DbLogTest.php location has changed. Also removed the changes introduced in the install file and DbLog.php.

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented
Issue tags: -Novice

Thanks @mitrpaka.

I think this makes much more readable DblogTest. #2 exposed the failing test. Not sure if we need an assertion in the absence of fatal errors what check this test.

dawehner’s picture

dawehner
Primary language German
commented 
+++ b/core/modules/dblog/tests/src/Kernel/DbLogControllerTest.php
@@ -17,11 +17,40 @@ class DbLogControllerTest extends KernelTestBase {
+  public function testNonLatinCharacters() {
+
+    $link = 'hello-
+      科州的小九寨沟绝美高山湖泊酱凉拌素鸡照烧鸡黄玫瑰
+      科州的小九寨沟绝美高山湖泊酱凉拌素鸡照烧鸡黄玫瑰
+      科州的小九寨沟绝美高山湖泊酱凉拌素鸡照烧鸡黄玫瑰
+      科州的小九寨沟绝美高山湖泊酱凉拌素鸡照烧鸡黄玫瑰
+      科州的小九寨沟绝美高山湖泊酱凉拌素鸡照烧鸡黄玫瑰
+      科州的小九寨沟绝美高山湖泊酱凉拌素鸡照烧鸡黄玫瑰
+      科州的小九寨沟绝美高山湖泊酱凉拌素鸡照烧鸡黄玫瑰
+      科州的小九寨沟绝美高山湖泊酱凉拌素鸡照烧鸡黄玫瑰
+      科州的小九寨沟绝美高山湖泊酱凉拌素鸡照烧鸡黄玫瑰
+      科州的小九寨沟绝美高山湖泊酱凉拌素鸡照烧鸡黄玫瑰
+      科州的小九寨沟绝美高山湖泊酱凉拌素鸡照烧鸡黄玫瑰
+      科州的小九寨沟绝美高山湖泊酱凉拌素鸡照烧鸡黄玫瑰
+      科州的小九寨沟绝美高山湖泊酱凉拌素鸡照烧鸡黄玫瑰
+      科州的小九寨沟绝美高山湖泊酱凉拌素鸡照烧鸡黄玫瑰';
+
+    \Drupal::logger('my_module')->warning('test', ['link' => $link]);
+  }
+

I think we should have some form of assertion here. Anything we can come up with?

dagmar’s picture

dagmar
Primary language Spanish
Location Argentina
commented
StatusFileSize
new 2882525-8.patch6.25 KB
new interdiff-2882525-5-8.txt787 bytes

Actually it is quite simple to test, since now link is a of type text the full link should be persisted in the database.

dawehner’s picture

dawehner
Primary language German
commented
Status: Needs review » Reviewed & tested by the community

Nice!

  • larowlan committed 06848b2 on 8.4.x 
    Issue #2882525 by dagmar, mitrpaka: Move suspicious string in DblogTest...
larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Reviewed & tested by the community » Fixed

Thanks

Fixed on commit

diff --git a/core/modules/dblog/tests/src/Kernel/DbLogControllerTest.php b/core/modules/dblog/tests/src/Kernel/DbLogControllerTest.php
index 8d650a7..ce7490f 100644
--- a/core/modules/dblog/tests/src/Kernel/DbLogControllerTest.php
+++ b/core/modules/dblog/tests/src/Kernel/DbLogControllerTest.php
@@ -47,11 +47,11 @@ public function testNonLatinCharacters() {
     \Drupal::logger('my_module')->warning('test', ['link' => $link]);
 
     $log = \Drupal::database()
-             ->select('watchdog', 'w')
-             ->fields('w', ['link'])
-             ->condition('link', '', '<>')
-             ->execute()
-             ->fetchField();
+      ->select('watchdog', 'w')
+      ->fields('w', ['link'])
+      ->condition('link', '', '<>')
+      ->execute()
+      ->fetchField();
 
     $this->assertEquals($log, $link);
   }

Committed as 06848b2 and pushed to 8.4.x.

Status: Fixed » Closed (fixed)

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