Fix duplicate word typos (the the, to to, etc) for test assertions

@jungle Thanks for opening this.

https://www.drupal.org/files/issues/2020-03-23/3121362-13-9.0.x.patch actually contains no fixes for deprecation notices, since all the wonky deprecation messages were already removed from the 9.0.x branch. ;)

It seems there's nothing to do here. Should we just close this as "by design"?

@jungle I replied upstream. I know #27 in the latest patch there. I was specifically looking at earlier patches since I believe all of that should be fixed in 9.0.x, and is in scope in the parent issue. I don't think any of https://www.drupal.org/files/issues/2020-03-26/interdiff-9.0.x-19-27.txt needs to only happen in 9.1.x, and all that can be done in both 9.1.x and 9.0.x. But I'm probably wrong (as usual) about what's in scope where, so thanks for opening this, just in case... ;) /shrug

p.s. To clarify, because none of the linked interdiff includes any of "for exception and deprecation messages" per the title here...

Per #3121362-38: Fix duplicate word typos (the the, to to, etc.) for code comments, the proper scope for this issue is fixing the duplicate words in the test assertions. More accurate title and minor summary updates.

git apply -R interdiff-9.0.x-19-27.txt
git diff > 3122547-7.patch

;)

Both affected tests pass locally. Applies cleanly to 9.0.x and 9.1.x. Doesn't apply to 8.9.x, but I get the sense this isn't going to be backported that far, so probably it doesn't matter.

Crediting @jungle since it's their code, not mine.

Thanks for the patch.

1) I reviewed the changes and see them as follows:

• +++ b/core/modules/aggregator/tests/src/Kernel/AggregatorPluginManagerTest.php
  @@ -35,7 +35,7 @@ public function testParserInfoAlter() {
  -    $this->assertTrue($widget_definition['definition_altered'], "The 'aggregator_test_parser' plugin definition was updated in in `hook_aggregator_parser_info_alter()`");
  +    $this->assertTrue($widget_definition['definition_altered'], "The 'aggregator_test_parser' plugin definition was updated in `hook_aggregator_parser_info_alter()`");
  

  "in in" => "in"

• +++ b/core/modules/aggregator/tests/src/Kernel/AggregatorPluginManagerTest.php
  @@ -45,7 +45,7 @@ public function testProcessorInfoAlter() {
  -    $this->assertTrue($widget_definition['definition_altered'], "The 'aggregator_test_processor' plugin definition was updated in in `hook_aggregator_processor_info_alter()`");
  +    $this->assertTrue($widget_definition['definition_altered'], "The 'aggregator_test_processor' plugin definition was updated in `hook_aggregator_processor_info_alter()`");
  

  Ditto.

• +++ b/core/modules/system/tests/modules/js_message_test/js/js_message_test.es6.js
  @@ -51,7 +51,7 @@
  -                `This is a message of the type, ${type}. You be the the judge of its importance.`,
  +                `This is a message of the type, ${type}. You be the judge of its importance.`,
  

  "the the" => "the"

• +++ b/core/modules/system/tests/modules/js_message_test/js/js_message_test.es6.js
  @@ -71,7 +71,7 @@
  -                }. You be the the judge of its importance.`,
  +                }. You be the judge of its importance.`,
  

  Ditto.

• +++ b/core/modules/system/tests/modules/js_message_test/js/js_message_test.js
  @@ -36,7 +36,7 @@
  -          messageObjects[area].indexes[type].push(message.add("This is a message of the type, ".concat(type, ". You be the the judge of its importance."), {
  +          messageObjects[area].indexes[type].push(message.add("This is a message of the type, ".concat(type, ". You be the judge of its importance."), {
  

  Ditto++.

• +++ b/core/modules/system/tests/modules/js_message_test/js/js_message_test.js
  @@ -45,7 +45,7 @@
  -          messageObjects.multiple.push(messageObjects.default.zone.add("This is message number ".concat(i, " of the type, ").concat(testMessages.types[i % testMessages.types.length], ". You be the the judge of its importance."), {
  +          messageObjects.multiple.push(messageObjects.default.zone.add("This is message number ".concat(i, " of the type, ").concat(testMessages.types[i % testMessages.types.length], ". You be the judge of its importance."), {
  

  Ditto ditto!

• +++ b/core/tests/Drupal/FunctionalJavascriptTests/Core/JsMessageTest.php
  @@ -50,8 +50,8 @@ public function testAddRemoveMessages() {
  -        $web_assert->elementContains('css', $selector, "This is a message of the type, $type. You be the the judge of its importance.");
  -        $current_messages[$selector] = "This is a message of the type, $type. You be the the judge of its importance.";
  +        $web_assert->elementContains('css', $selector, "This is a message of the type, $type. You be the judge of its importance.");
  +        $current_messages[$selector] = "This is a message of the type, $type. You be the judge of its importance.";
  

  Ditto ditto ditto.

• +++ b/core/tests/Drupal/FunctionalJavascriptTests/Core/JsMessageTest.php
  @@ -69,7 +69,7 @@ public function testAddRemoveMessages() {
  -      $current_messages[] = "This is message number $i of the type, {$types[$i % count($types)]}. You be the the judge of its importance.";
  +      $current_messages[] = "This is message number $i of the type, {$types[$i % count($types)]}. You be the judge of its importance.";
  

  And final one.

2) Patch applied cleanly:

[mac:kristen:drupal-9.1.x-dev]$ patch -p1 < 3122547-7.patch 
patching file core/modules/aggregator/tests/src/Kernel/AggregatorPluginManagerTest.php
patching file core/modules/system/tests/modules/js_message_test/js/js_message_test.es6.js
patching file core/modules/system/tests/modules/js_message_test/js/js_message_test.js
patching file core/tests/Drupal/FunctionalJavascriptTests/Core/JsMessageTest.php

3) Searched the code after applying the patch for "the the" and they were all in comments:

• core/lib/Drupal/Component/FileSystem/RegexDirectoryIterator.php
• core/lib/Drupal/Core/Installer/Form/SelectProfileForm.php
• core/lib/Drupal/Core/Test/TestDiscovery.php
• core/modules/content_moderation/src/Plugin/Field/FieldWidget/ModerationStateWidget.php
• core/modules/jsonapi/src/Query/EntityConditionGroup.php
• core/modules/jsonapi/tests/src/Functional/JsonApiFunctionalMultilingualTest.php
• core/modules/jsonapi/tests/src/Functional/ResourceTestBase.php
• core/modules/language/src/Plugin/LanguageNegotiation/LanguageNegotiationContentEntity.php
• core/modules/path_alias/tests/src/Kernel/AliasTest.php
• core/modules/system/tests/src/Functional/System/SitesDirectoryHardeningTest.php
• core/modules/system/tests/src/FunctionalJavascript/Form/TriggeringElementTest.php
• core/modules/taxonomy/tests/src/Kernel/Migrate/d6/MigrateVocabularyFieldInstanceTest.php
• core/modules/views/tests/src/FunctionalJavascript/PaginationAJAXTest.php
• core/modules/views/tests/src/Kernel/Handler/FieldCounterTest.php
• core/modules/workspaces/src/EntityQuery/Query.php
• core/tests/Drupal/Tests/Component/Render/FormattableMarkupTest.php
• core/themes/claro/css/components/dropbutton.css
• core/themes/claro/css/components/dropbutton.pcss.css

4) Searched the code after applying the patch for "in in" and there weren't any left.

Looks good so marking RTBC.

Committed 8964921 and pushed to 9.1.x. Thanks!

@catch Maybe there is a better place to ask and maybe it'll show up at some point, but I noticed that 2 other fixed issues showed up in my issue credits but this one didn't. The odd thing is that one that did show up was marked fixed *after* this issue.

https://www.drupal.org/u/Kristen%20Pol/issue-credits/3060

If the issue credit for this issue doesn't show up at some point, should I file an issue somewhere since it may be a general issue? I don't want to appear "needy" ;) but am concerned that this might be an issue for other people too. The credit does show up for dww and jungle:

https://www.drupal.org/u/dww/issue-credits/3060
https://www.drupal.org/u/jungle/issue-credits/3060

so I would assume it's due to:

1) A caching issue and it will show up at some point
2) The fact it's the last name in the list and has ":" at the end of it (unlikely)
3) The fact there is a space in the name (unlikely)
4) Something else entirely

Thanks!

@Kristen Pol re: #12:

No, it's due to the fact that the issue credit system isn't based on Git commits at all, but is entirely based on checkboxes on the issue nodes themselves. @catch seems to have credited you for the review during commit, but didn't actually save that change on the issue. Only project maintainers can toggle those checkboxes and have the value "stick", but they have to save a comment on the issue for that to happen. Since I'm a project maintainer here, and since @catch clearly intended to credit you, doing so now.

Cheers,
-Derek

@dww Oh, that totally makes sense, it's just the checkboxes. :) My brain obviously hasn't woken up yet. Thanks for sorting that out!

Side note, from what I've seen in the past, if you add a *file* then it automatically checks the box for you but, when I review, I don't add files just for the sake of adding files to make the checkbox get checked... only if there are screenshots from manual testing.

UPDATE: Maybe the logic could be changed that if someone marks it RTBC then it also is automatically checked?

@catch: @alexpott cherry-picked #3121362: Fix duplicate word typos (the the, to to, etc.) for code comments all the way back to 8.8.x. Is this worth backporting, too? That'd keep tests/assertions more similar on all branches to make further patching easier and not require manual backporting. If so, here's a patch that applies cleanly to 8.8.x and 8.9.x. There was a conflict in core/modules/system/tests/modules/js_message_test/js/js_message_test.js but everything else applied cleanly and I just had to run yarn build:js to regenerate that .js file. Interdiff can't hang since patch #7 doesn't apply on this branch, so here's a raw diff of the 2 patch files. /shrug

@Kristen Pol: Generally, core committers are pretty on it. It's rare that there's a mix-up like this, and when it happens, they usually resolve it very quickly. I'm hesitant to propose any further changes to the behavior of the credit system. There was a few month period where any comment on an issue automatically checked the box for you, and while that was live, there was a noticeable spike in useless comments on core issues where folks were just fishing for credit. Even if we restrict it to only RTBC'ing comments, I fear folks will start leaving useless-RTBC comments (when something isn't really RTBC, or they haven't really reviewed). Of course, all this is totally out of scope in this issue. ;) If you're serious, you should propose it in the core ideas queue or perhaps directly in the drupal.org customizations queue and try to get support there.

Thanks,
-Derek

@dww Thanks for the info. Yeah, makes sense. Lots of "gaming the system" we have to consider :(

@dww Do you want a review of the 8.9 patch?

@jungle Thanks for jumping on that! All good. :)

Re: crediting, @Kristen Pol was in the commit message, so if her name got unchecked, it was due to a data loss race condition with System Message. If that happens again in the future, it should definitely be reported to infra as soon as it happens. As an aside, note that the fact that the issue was reopened effectively "de-activated" those credits, since credit is only applied when the issue is Fixed or Closed (Fixed). :)

I reviewed the backport with git diff --color-words, committed it to 8.9.x, and cherry-picked it to 8.8.x. Thanks!