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
Some basic functionality is missing test coverage.
Proposed resolution
Create tests for
- unsubscribing aka disabling
- tokens
- user preferences
- user/node/comment deletion
- mail body
- extra field display
settings form
I don't think we want to create all these tests at once, it's more helpful to commit them in parts. I think unsubscribing, tokens and user preferences are top priority, because they are basic non-admin features, and the rest can be done in separate issues?
Remaining tasks
User interface changes
API changes
Data model changes
| Comment | File | Size | Author |
|---|---|---|---|
| #13 | comment_notify-n2618192-13.interdiff.txt | 405 bytes | DamienMcKenna |
| #13 | comment_notify-n2618192-13.patch | 24.8 KB | DamienMcKenna |
| #11 | interdiff-8-11.txt | 7.93 KB | gnuget |
| #11 | 2618192-basic-coverage-11.patch | 24.83 KB | gnuget |
| |||
| #8 | 2618192-basic-coverage-interdiff-7-8.txt | 891 bytes | gnuget |
Comments
Comment #2
gnugetI've been using this module for a few days now and I already found a few issues on it (see [#2853329 and #2850935: Error when the user has been created and edit his profile for first time) I already provided a patch for those but I would like to help stabilizing this module.
For start moving this I just attached a patch with the following changes:
I will continue working on this, and like the description mentioned, Would be great to start commiting this and create separate issues for the rest of the tests, so I can work on the tests of (see [#2853329 and #2850935: Error when the user has been created and edit his profile for first time) directly in these issues.
Comment #3
gnugetComment #5
gnugetIt seems to the group is required, lets try again
Comment #7
gnugetok, lets try again.
Comment #8
gnugetIt seems to I forgot to add the group annotation in the base class.
Let's see if I can make happy the testbot this time.
Comment #9
greggles@gnuget thanks for your work on improving test coverage. I wonder if you can comment on #2851055: Update documentation about anonymous email field?
Comment #10
gnugetSure thing!
Comment #11
gnugetNew patch, this one includes tests for the settings form.
Comment #12
ArlaLooks good to me. Having the test converted and extended for the settings form is a great start. As said in IS, we could commit this and either keep the issue open or close it and create followups for each area to extend tests for. Thus I'm marking RTBC.
Just noting that 1) a prettier diff could be made with
git diff -M2and 2) the commentExists() helper is not used (and was not used before this patch either) (I don't really mind it hanging around anyway).Comment #13
DamienMcKennaOne small thing - the abstract class shouldn't have a @group.
Comment #14
DamienMcKennaComment #15
gnugetThanks!
Comment #16
DamienMcKennaIt might be worthwhile to use e.g. Maillog with the tests to make it easier to see what emails are being generated.
Comment #18
gregglesThis seems like an important thing to get merged to help with other work on the module. Thanks so much for your help!
I marked this issue as fixed and created a followup at #3020400: Basic test coverage round 2.