- Issue created by @dww
- πΊπΈUnited States dww
This is going to conflict with π Correct & improve update status email subject Postponed , let's land that, first.
- πΊπΈUnited States dww
Per Slack with @xjm, expanding scope here to actually fix
UpdateMailTest
on its own, before trying to add more coverage to it at π Correct & improve update status email subject Postponed . - πΊπΈUnited States dww
I believe this is ready. The test is passing locally, so if the pipeline isn't green, it's due to a random fail. π
- πΊπΈUnited States smustgrave
For this one I actually did the side-by-side review as it was easier then inline.
The matrix of core and contrib hitting every combo of current, not current, not secure, secure appear to be hit.
Question do we need coverage for REVOKED and NOT_SUPPORTED?
- πΊπΈUnited States dww
Thanks for the review!
Good question about the other statuses. "need" is a strong word. π I'm slightly worried that every possible combination of all possible status values for core vs. contrib will explode the size of this data provider. It's not like the logic in
_update_message_text()
is all that complicated. Seems plausible that we don't need to test every possible code path in there. This MR is already larger than the "combined" MR that prompted splitting this off to a separate issue. π I don't want to explode the scope here more than I already have.I vote we get this in ASAP as a significant improvement to what we have now (and to unblock the bugfix that prompted this). If we decide we need more cases later, we can always add them easily enough.
Thanks,
-Derek