Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
Let's fix some tests after #2735927: Workbench moderation + field encrypt + translations isn't working got in
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Problem/Motivation
Proposed resolution
Remaining tasks
User interface changes
API changes
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#3 | fix_tests_round_2-2764851-3.patch | 4.24 KB | talhaparacha |
#2 | 2764851-2.patch | 1.84 KB | dawehner |
Comments
Comment #2
dawehnerComment #3
talhaparacha CreditAttribution: talhaparacha commentedThank you @dawehner for the patch in #2. As per my review, it fixed the FieldEncryptCacheTest and FieldEncryptProcessEntitiesTest but FieldEncryptTest and EncryptedFieldValueManagerTest were still failing.
Here is another patch which combines all the changes from the patch in #2 along with some additions, and fixes all the tests. It also brings the module back to a stable condition.
Since the module is currently broken (as can be inferred from #2779853: EntityStorageException: Missing bundle for entity type node), I’m raising the priority for this issue.
Comment #4
talhaparacha CreditAttribution: talhaparacha commentedThe additional changes in the patch in #3 are explained below:
1. We need to assert for 10 encrypted field values and not 5 in testEncryptFieldTranslation(). That’s because there should be 5 encrypted values for English text along with 5 encrypted values for French text.
2. Typo correction.
3. No need for grouping an abstract class.
4. In #2744887: Fix automatic tests this piece of code:
was changed to this for simplification purposes:
But on a closer analysis it can be seen that these two code pieces are NOT equivalent. More specifically, the former can return NULL but the latter will never return NULL. This change was causing failures in testEncryptFieldRevision() and hence I’ve tried to correct it.
5. Additional mocking required because of the change mentioned in last point.
Comment #6
talhaparacha CreditAttribution: talhaparacha commentedBecause testbot doesn't pull dev versions, but rather "stable" releases so Encrypt module dependency wasn't met.
Comment #7
colanWorks well in my testing.
Comment #9
nerdsteinPushed to 8.x-2.x