Add test cases for cloud_server_template module

CommentFileSizeAuthor
#7 3012975_20181113.patch57.76 KBXiaohua Guan
#2 3012975_20181112.patch37.35 KBXiaohua Guan
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Xiaohua Guan created an issue. See original summary.

Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented
FileSize
3012975_20181112.patch37.35 KB
Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented
Status: Active » Needs review
Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented

@yas

Please review the patch file.

baldwinlouie’s picture

baldwinlouie CreditAttribution: baldwinlouie commented

@xioahua-guan, Thank you for the patch. I reviewed this and have the following feedback. When you refactored drupal_set_message, I think it is better to use dependency injection to inject the messenger object into the Form class rather than call the global messenger service using \Drupal::messenger().

Please take a look at AwsCloudContentForm.php's create method on how to set up the messenger object. Then you can look at KeyPairImportForm.php on how to use the messenger object.

What do you think?

Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented

@baldwinlouie

Thanks for your code review and comment.

> What do you think?
I agree with your way. I will do some modification.

Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented
FileSize
3012975_20181113.patch57.76 KB
Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented
Xiaohua Guan’s picture

Xiaohua Guan CreditAttribution: Xiaohua Guan commented

@baldwinlouie

Hi, I attached a new patch file, 3012975_20181113.patch. Please take a look. Thanks.

yas’s picture

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

@xiaohua-guan Thank you for your patch. It looks good to me.

@baldwinlouie, how do you think?

baldwinlouie’s picture

baldwinlouie CreditAttribution: baldwinlouie commented

@xiaohua-guan Looks good! Thanks for the patch.

  • yas committed efd8541 on 8.x-1.x authored by Xiaohua Guan 
    Issue #3012975 by Xiaohua Guan, baldwinlouie, yas: Add SimpleTest for...
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

@xiaohua-guan,
@baldwinlouie

Thank you for your efforts.

Status: Fixed » Closed (fixed)

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