Problem/Motivation

In preparation for fixing the language context problems revealed in #3263463: Cannot add language to send mail action and #2471481-161: Integrate Typed Data Widgets the language test coverage needs to be improved. Specifically the two units tests for actions that have a language context currently only test with the site default language. This means that the language manipulation in those actions doEvaluate functions is not covered by tests. Faults can be introduced and no tests would fail.

Proposed resolution

In SystemSendEmailTest and SystemEmailToUsersOfRoleTest add a language which is not the site default, and alter one of the tests in each of these classes mentioned to use this instead of defaulting.

Issue fork rules-3264444

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jonathan1055 created an issue. See original summary.

jonathan1055’s picture

jonathan1055 commented
Status: Active » Needs review

This first commit adds a non-default language, Danish, into the two tests. The assertions are not changed (yet) so this should fail, and prove that the modified tests are working.

jonathan1055 opened merge request !8

tr’s picture

tr commented

OK, this is a start. Can you do the same for everywhere we use a language context variable? There are some path alias conditions and actions, in addition to these two email actions.

jonathan1055’s picture

jonathan1055 commented

Yes, there are 3 actions that use a Language context variable, the two e-mail ones listed above and PathAliasCreate. The unit tests for PathAliasCreate already have a language entered - see PathAliasCreateTest::testActionExecutionWithLanguage

Likewise there are two conditions that use a language, and these are already covered:
PathAliasExistsTest - both tests already have language 'en', each test checks with and without a language.
PathHasAliasTest - already uses language 'en', checks with and without a language.

So it is only the two email actions that have no language context variable in the tests. Maybe I was a little unclear in the issue summary, I did not mean that we specifically need to use a language other than English, what I meant was that those two tests pass no language variable at all, so any change to the language context definition would not be covered. The additions above are enough to fail the test if the language definition is done incorrectly (I know, because I coded one badly and got no test failures, that's how I discovered it :-)

tr’s picture

tr commented

Sounds good, I'll commit it later today

jonathan1055’s picture

jonathan1055 commented
Status: Needs review » Reviewed & tested by the community

Thanks

tr’s picture

tr commented

I got sidetracked by the D10 test failure seen above, which I'm handling in #3265360: [D10] Test failure. It has nothing to do with this patch, but I'd like to have the tests running green first.

  • TR committed d1531c9 on 8.x-3.x authored by jonathan1055 
    Issue #3264444 by jonathan1055: Include language settings in email tests
tr’s picture

tr commented
Status: Reviewed & tested by the community » Fixed

The D10 test failure is gong to take a while to resolve, so I'm not going to wait. Committed.

Status: Fixed » Closed (fixed)

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