CommentFileSizeAuthor
#9 interdiff-3165907-9.txt1.16 KBjigish.addweb
#9 3165907-9.patch9.64 KBjigish.addweb
#8 interdiff-3165907-8.txt2.46 KBjigish.addweb
#8 3165907-8.patch9.63 KBjigish.addweb
#5 interdiff-3165907-5.txt4.96 KBjigish.addweb
#5 3165907-5.patch8.34 KBjigish.addweb
#2 3165907-2.patch6.66 KBjigish.addweb
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jigish.addweb created an issue. See original summary.

jigish.addweb’s picture

jigish.addweb CreditAttribution: jigish.addweb at AddWeb Solution Pvt. Ltd. commented
Issue summary: View changes
FileSize
3165907-2.patch6.66 KB
jigish.addweb’s picture

jigish.addweb CreditAttribution: jigish.addweb at AddWeb Solution Pvt. Ltd. commented
Status: Active » Needs review
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Status: Needs review » Needs work 
  1. +++ b/modules/cloud_service_providers/aws_cloud/tests/src/Functional/cloud/config/AwsCloudConfigTest.php
@@ -324,4 +337,128 @@ class AwsCloudConfigTest extends CloudConfigTestBase {
+    $target_region_index = random_int(0, count($regions) - 1);
+    $target_region_display_name = $regions[array_keys($regions)[$target_region_index]];

    Can we use an array_rand function here? (Please search array_rand($regions) in the enter source code as a reference.) 

  2. +++ b/modules/cloud_service_providers/aws_cloud/tests/src/Functional/cloud/config/AwsCloudConfigTest.php
@@ -324,4 +337,128 @@ class AwsCloudConfigTest extends CloudConfigTestBase {
+          $add[$i]["regions[]"][] = $region_name;

    Change to $add[$i]['regions[]'][] 

  3. +++ b/modules/cloud_service_providers/aws_cloud/tests/src/Functional/cloud/config/AwsCloudConfigTest.php
@@ -324,4 +337,128 @@ class AwsCloudConfigTest extends CloudConfigTestBase {
+      $this->assertSession()->statusCodeEquals(200);

    Change to $this->assertNoErrorMessage();

jigish.addweb’s picture

jigish.addweb CreditAttribution: jigish.addweb at AddWeb Solution Pvt. Ltd. commented
FileSize
3165907-5.patch8.34 KB
interdiff-3165907-5.txt4.96 KB
jigish.addweb’s picture

jigish.addweb CreditAttribution: jigish.addweb at AddWeb Solution Pvt. Ltd. commented
Status: Needs work » Needs review

@yas Thank you for reviewing the patch.

For 3rd point, We can not change to $this->assertNoErrorMessage(); due to the error message, The module Google Applications is invalid. Please enable the module..

I added same comment as below

   // @FIXME: Not refactored due to the error message,
   // The module Google Applications is invalid. Please enable the module.
   $this->assertSession()->statusCodeEquals(200);

Please review the updated patch.

Thanks

yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Status: Needs review » Needs work

@jigishaddweb

Thank you for the update.

  1. +++ b/modules/cloud_service_providers/aws_cloud/tests/src/Functional/cloud/config/AwsCloudConfigTest.php
@@ -324,4 +337,129 @@ class AwsCloudConfigTest extends CloudConfigTestBase {
+      // @FIXME: Not refactored due to the error message,
+      // The module Google Applications is invalid. Please enable the module.
+      $this->assertSession()->statusCodeEquals(200);

    The comment is good. So can we use $this->assertWarningMessage() here? We can change the comment as follows:

          // @FIXME: We don't use assertNoErrorMessage(), since it displays
      // "The module Google Applications is invalid. Please enable the module."
  2. Also, please fix the coding standard violation at https://www.drupal.org/pift-ci-job/1795460
jigish.addweb’s picture

jigish.addweb CreditAttribution: jigish.addweb at AddWeb Solution Pvt. Ltd. commented
FileSize
3165907-8.patch9.63 KB
interdiff-3165907-8.txt2.46 KB
jigish.addweb’s picture

jigish.addweb CreditAttribution: jigish.addweb at AddWeb Solution Pvt. Ltd. commented
FileSize
3165907-9.patch9.64 KB
interdiff-3165907-9.txt1.16 KB
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Status: Needs work » Reviewed & tested by the community

@jigishaddweb

Thank you for adding the test case. It looks good to me. I'll merge the patch to 8.x-1.x, 8.x-2.x and 3.x and close this issue as Fixed.

  • yas committed 2a5556a on 8.x-1.x authored by jigish.addweb 
    Issue #3165907 by jigish.addweb, yas: Add test cases for cloud service...

  • yas committed a4f846d on 8.x-2.x authored by jigish.addweb 
    Issue #3165907 by jigish.addweb, yas: Add test cases for cloud service...

  • yas committed 2ad6725 on 3.x authored by jigish.addweb 
    Issue #3165907 by jigish.addweb, yas: Add test cases for cloud service...
yas’s picture

yas
🇯🇵 he/him
Primary language Japanese
Location California 🇺🇸
CreditAttribution: yas at DOCOMO Innovations, Inc. commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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