Problem/Motivation

Simple tests will likely require common functionality, such as generating a user with necessary permissions and generating test keys. We should define a base class to share that functionality.

Proposed resolution

Add a EncryptTestBase class with common functionality. EncryptTest should extend the base class.

CommentFileSizeAuthor
#11 2692387-11.patch605 bytessvendecabooter
#7 2692387-7.patch6.44 KBsvendecabooter
#5 2692387-5.patch6.4 KBsvendecabooter
#4 2692387-4.patch6.02 KBrlhawk
#2 2692387-2.patch6.01 KBrlhawk
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

rlhawk created an issue. See original summary.

rlhawk’s picture

rlhawk
Primary language English
Location Seattle, Washington, United States
CreditAttribution: rlhawk commented
Status: Active » Needs review
FileSize
2692387-2.patch6.01 KB

Here's a patch that adds a base class and extends it.

Status: Needs review » Needs work

The last submitted patch, 2: 2692387-2.patch, failed testing.

rlhawk’s picture

rlhawk
Primary language English
Location Seattle, Washington, United States
CreditAttribution: rlhawk commented
Status: Needs work » Needs review
FileSize
2692387-4.patch6.02 KB

New patch to address failing test.

svendecabooter’s picture

svendecabooter
Primary language Dutch
Location Gent
CreditAttribution: svendecabooter at Sven Decabooter for Acquia commented
FileSize
2692387-5.patch6.4 KB

This could be useful for the field_encrypt module, which currently creates keys and encryption profiles as part of the general setup.
Instead, the field_encrypt module tests could extend EncryptTestBase, and leverage the setup that's done there.

I have updated this patch:
* The creation of keys can be done programmatically, rather then through the UI, because the UI case is already covered by Key module's KeyServiceTest.
* 2 test encryption profiles are being created programmatically by EncryptTestBase. They are not used in EncryptTest, because that test tests the addition of encryption profiles through the UI, and sets assertions for that. Instead, this setup logic is aimed for other tests extending the base test, such as the aforementioned field_encrypt tests.

Status: Needs review » Needs work

The last submitted patch, 5: 2692387-5.patch, failed testing.

svendecabooter’s picture

svendecabooter
Primary language Dutch
Location Gent
CreditAttribution: svendecabooter at Sven Decabooter for Acquia commented
Status: Needs work » Needs review
FileSize
2692387-7.patch6.44 KB

Update for failing test.

rlhawk’s picture

rlhawk
Primary language English
Location Seattle, Washington, United States
CreditAttribution: rlhawk commented
Status: Needs review » Reviewed & tested by the community

Terrific, thanks. The only issue I came across was that simpletest attempted to instantiate the abstract base class, resulting in an error. I removed the base class from the encrypt @group and now it's all good.

  • rlhawk committed ade98e0 on 8.x-3.x authored by svendecabooter 
    Issue #2692387 by rlhawk, svendecabooter: Define and use base class for...
rlhawk’s picture

rlhawk
Primary language English
Location Seattle, Washington, United States
CreditAttribution: rlhawk commented
Status: Reviewed & tested by the community » Fixed
svendecabooter’s picture

svendecabooter
Primary language Dutch
Location Gent
CreditAttribution: svendecabooter at Sven Decabooter for Acquia commented
Status: Fixed » Needs review
FileSize
2692387-11.patch605 bytes

I spotted an error in the definition of the 256 bit key. Probably doesn't result into an error, since 256 is the default value.
Patch attached to clean this up.

rlhawk’s picture

rlhawk
Primary language English
Location Seattle, Washington, United States
CreditAttribution: rlhawk commented
Status: Needs review » Reviewed & tested by the community

Thanks.

  • rlhawk committed 7b5fbcf on 8.x-3.x authored by svendecabooter 
    Issue #2692387 by svendecabooter, rlhawk: Define and use base class for...
rlhawk’s picture

rlhawk
Primary language English
Location Seattle, Washington, United States
CreditAttribution: rlhawk commented
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

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