We've been having some strange issues around tests not being able to download the modules needed for the tests to complete, so we want to try being more explicit about which modules are necessary for testing.

Our dependencies are:

  • tfa
  • key
  • encrypt
  • encrypt_test
CommentFileSizeAuthor
#3 2937043-3-test-deps.patch1.24 KBdaggerhart
#10 2937043-10.patch284 bytesdaggerhart
#2 2937043-2-test-deps.patch353 bytesdaggerhart

Comments

daggerhart created an issue. See original summary.

daggerhart’s picture

daggerhart commented
Status: Active » Needs review
StatusFileSize
new 2937043-2-test-deps.patch353 bytes

Adding test_dependencies to info file.

daggerhart’s picture

daggerhart commented
Issue summary: View changes
Status: Needs review » Postponed
StatusFileSize
new 2937043-3-test-deps.patch1.24 KB

In slack we questioned the need for the "node" module as a test dependency. I reviewed the tests for node-related work, removed the dependency and found that the tfa route for the EntryForm requires "access content" permission.

Researched it a bit more and determined that #2930590: Review access requirements needs to land before the ga_login module can remove the node dependency. Attached is the patch that should work after #2930590 lands, but I'm postponing this issue until then.

daggerhart’s picture

daggerhart commented
Status: Postponed » Needs review

Status: Needs review » Needs work

The last submitted patch, 3: 2937043-3-test-deps.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

nerdstein’s picture

nerdstein
he/him
Primary language English
Location United States
commented

There is a thought that the test dependencies need to be committed, as we're getting the following message:

Line 40 of modules/contrib/ga_login/src/Tests/GALoginTestBase.php:
Unable to install modules tfa, node, encrypt, encrypt_test, key, ga_login due to missing modules tfa, encrypt, encrypt_test, key.

I'm going to commit to see if it resolves but not mark this as fixed yet.

  • nerdstein committed 28a2925 on 8.x-1.x authored by daggerhart 
    Issue #2937043 by daggerhart: Add test_dependencies: to ga_login.info....
nerdstein’s picture

nerdstein
he/him
Primary language English
Location United States
commented
Status: Needs work » Fixed

Test dependencies are now loading in the module, which resolved an error. There are more errors now, but this is a separate issue. Marking as fixed. Thanks, @daggerhart

Status: Fixed » Closed (fixed)

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

daggerhart’s picture

daggerhart commented
Status: Closed (fixed) » Needs review
StatusFileSize
new 2937043-10.patch284 bytes

Back to this issue, I believe we want to have ga_login use the tfa 1.x-dev branch for testing. This will hopefully allow us to manage our release process better.

This is a new patch that only changes the required version of tfa as a test dependency of ga_login.

daggerhart’s picture

daggerhart commented
Status: Needs review » Fixed

Closing this issue. It's done. The last patch wasn't necessary and the real problem was solved and explained here: https://www.drupal.org/project/ga_login/issues/2951323#comment-12517373

Status: Fixed » Closed (fixed)

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