Extend unit test coverage for LanguageNegotiationContentEntity

Thanks @stefanos.petrakis for working on this!

1. 3 coding standards messages

   core/modules/language/tests/src/Unit/Plugin/LanguageNegotiation/LanguageNegotiationContentEntityTest.php
   line 10 Unused use statement
   line 14 Unused use statement
   line 17 Unused use statement

2. +++ b/core/modules/language/tests/src/Unit/Plugin/LanguageNegotiation/LanguageNegotiationContentEntityTest.php
   @@ -0,0 +1,138 @@
   +  protected function setUp() {
   

   See #3107732: Add return typehints to setUp/tearDown methods in concrete test classes, return type hinting is expected in 9.x now,

   -  protected function setUp() {
   + protected function setUp(): void {
   

3. +++ b/core/modules/language/tests/src/Unit/Plugin/LanguageNegotiation/LanguageNegotiationContentEntityTest.php
   @@ -0,0 +1,138 @@
   +    $language_de = $this->createMock(LanguageInterface::class);
   +    $language_de->expects($this->any())
   

   Prefer using Prophecy for mocking, see https://www.drupal.org/docs/8/phpunit/using-prophecy, but it's ok to not use it. Non-mandatory.

4. +++ b/core/modules/language/tests/src/Unit/Plugin/LanguageNegotiation/LanguageNegotiationContentEntityTest.php
   @@ -0,0 +1,138 @@
   +    // Create a language manager stub.
   +    $language_manager = $this->getMockBuilder(ConfigurableLanguageManagerInterface::class)
   ...
   +    // Create a user stub.
   +    $this->user = $this->getMockBuilder(AccountInterface::class)
   

   Code here is self-explained. To me, comments are unnecessary, but it's ok to keep it if you like.

Switching to 9.1.x, could be backported to 8.x

+++ b/core/modules/language/tests/src/Unit/Plugin/LanguageNegotiation/LanguageNegotiationContentEntityTest.php
@@ -0,0 +1,133 @@
+    try {
+      $languageNegotiationContentEntity->getLangcode();
+    }
+    catch (Throwable $t) {
+      $this->assertEquals("Trying to get property of non-object", str_replace("'query' ", '', $t->getMessage()));
+    }
...
+    try {
+      $languageNegotiationContentEntity->getLangcode($request);
+    }
+    catch (Throwable $t) {
+      $this->assertEquals("Call to a member function getLanguages() on null", $t->getMessage());
+    }

To have try-catch in tests, not sure if it's against the best practice.

expectException() and expectExceptionMessage() are for that. but if doing so, breaking the one big test into small ones is necessary as in general, one exception is captured in one method. Correct me if I were wrong.

This issue is being reviewed by the kind folks in Slack, #need-reveiw-queue. We are working to keep the size of Needs Review queue [2700+ issues] to around 200

If I'm understanding #9 correct is this issue waiting on #3130751: Fix LanguageNegotiationContentEntity::getLangcode() errors thrown? If so this should be postponed.

Changes look good but the issue summary mentions tests for

• ::processOutbound()
• ::getLanguageSwitchLinks()

Which I don't see.

See tests were added for

::processOutbound()
::getLanguageSwitchLinks()

So removing the IS update tag.

Changes look good but do think this postpones #3130104: Add unit test coverage for LanguageNegotiationBrowser as we are now mixing tickets.

Needs a rebase.

Please review.

Going to mark it but going to ask @rpayanm why didn't you just update the existing MR? It was pointing to the correct branch?

Going to mark it but going to ask @rpayanm why didn't you just update the existing MR? It was pointing to the correct branch?

I tried to do it, but I found hard conflicts to solve, since the previous patch did not have a lot of changes I created a new branch. Sorry about that.

This looks fine and we can always add/refine to the improved tests later.

Committed/pushed to 11.x and cherry-picked to 10.1.x, thanks!