Fix test in MenuLinkParentTest::testTransformException

Back to NR after #3 turned out to be a random fail.

+++ b/core/modules/migrate/tests/src/Unit/process/MenuLinkParentTest.php
@@ -88,7 +88,7 @@ protected function setUp(): void {
   public function testTransformException(array $source_value) {
-    [$parent_id, $menu_name, $parent_link_path] = $source_value;

Part of me likes having the 3rd value in the input array because this is a process plugin that expects three values but yes, it isn't needed. The source data for this is from the dataprovider, providerTransformException. So, if we are only going to use two variables then the data provider should only provide two. Setting to NW to also update the data provider.

Please review

@naresh_bavaskar, thanks for the patch and interdiff.

+++ b/core/modules/migrate/tests/src/Unit/process/MenuLinkParentTest.php
@@ -108,9 +108,6 @@ public function providerTransformException() {
-      'parent link is an internal URI that does not exist' => [
-        'source_value' => [1, NULL, 'admin/structure'],
-      ],

This change removes a test case which we want to keep. Want needs to happen is to modify each test case to provide two parameters instead of three. That will avoid the unused variable.

The PHPUnit docs explain how dataProviders work.

Removing the third parameter from data provider and reverting the removed test case from #7

Yes, that looks correct.

I applied that patch and check the changed manually as well looking at the interdiff. There are no coding standard errors reported by testing.

This is good to go.

Thanks!

+++ b/core/modules/migrate/tests/src/Unit/process/MenuLinkParentTest.php
@@ -103,13 +103,13 @@ public function providerTransformException() {
       'parent link path/menu name not passed' => [
-        'source_value' => [1, NULL, 'http://drupal.org'],
+        'source_value' => [1, NULL],
       ],
       'parent link is an internal URI that does not exist' => [
-        'source_value' => [1, NULL, 'admin/structure'],
+        'source_value' => [1, NULL],
       ],

This is now really odd the source_value is the same for both tests. I guess this is why the previous patch removed it.

I think we need to check whether these tests are really testing what we think they are.

@alexpott, thanks. Yes, that isn't right.

This now fixes the test to do what it should have been doing all along. It now properly tests all three paths that can cause the exception to be thrown. No interdiff because this is so small.

Change the title to show that a test is being changed.

Hi, I've applied the patch in #13 and it applied cleanly.

Also verified that the values for 'source_value' are now different for the three cases.

So RTBC.

@sulfikar_s, thanks for reviewing this patch! Instead of attaching an image that you applied the patch locally it is OK to comment 'I applied the patch and .. then explain what you did in the review. The community know the patch will apply because the test bot ran the tests.

Committed and pushed b3d61bd368 to 9.2.x and b9d7fac590 to 9.1.x. Thanks!

Backported to 9.1.x as this is a test fix.

@sulfikar_s - thank you for looking into this issue. Posting screenshots of your codebase or CLI does not advance the issue, since the automated testing infrastructure tells us whether the patch applies correctly. So, I've removed the issue credit for that screenshot. In the future, you can get credit for issues by reading the issue to understand its purpose, and posting your review or testing of that purpose. Thank you!