Repository navigation
Add nth email, sender and recipient assertions - #130
Conversation
Every "last email" action gets an "nth email" counterpart. Positions start at 1 and count in the order the emails were received; the ...To variants only count emails sent to the given address. Add seeInLastEmailSender, seeInNthEmailSender, seeInLastEmailRecipient and seeInNthEmailRecipient, backed by Email::getSender(). The sender is an optional trailing constructor argument to keep existing callers working. Fix the message sort tie-breaker, which compared created_at and id as a concatenated string and therefore ordered id 10 before id 9 within the same second. Supersedes captbaritone#102. Resolves: captbaritone#12, captbaritone#98
Jamesking56
left a comment
There was a problem hiding this comment.
Please fix the recipient assertion so it passes only when an individual recipient contains the expected value. I confirmed the false positive in both the last and nth variants with a focused PHP reproduction.
|
|
||
| protected function seeInEmailRecipients(Email $email, string $expected): void | ||
| { | ||
| $this->assertStringContainsString($expected, implode(', ', $email->getRecipients()), "Email Recipients Contain"); |
There was a problem hiding this comment.
Medium — check each recipient separately. Joining the recipients allows a match to span recipient boundaries. For example, with ['<alice@example.com>', '<bob@example.com>'], an expectation of '<alice@example.com>, <bob@example.com>' passes even though neither recipient contains that string. This violates the documented ‘one recipient matches’ behavior and affects both seeInLastEmailRecipient() and seeInNthEmailRecipient(). A focused PHP reproduction confirmed that both assertions incorrectly pass. Please check each recipient separately and pass only when at least one contains the expected value. Add a regression test for an expectation that spans recipient boundaries.
There was a problem hiding this comment.
Good catch, thanks. Fixed in e54a249: seeInEmailRecipients now checks each recipient on its own and passes only if at least one contains the expected value. On failure the message still lists all recipients.
Regression tests:
- Unit:
testSeeInLastEmailRecipientDoesNotMatchAcrossRecipientsuses your'<alice@example.com>, <bob@example.com>'example. It failed on the old code. - Acceptance:
test_see_in_email_recipient_checks_each_recipientsends one real email to two recipients. It checks that a single recipient matches and that a value spanning both fails throughseeInNthEmailRecipient.
1019fa4 adds a test that nth positions follow created_at before the id tie-breaker. It also notes in the README that addresses match by substring, the same way the existing ...To actions already work.
Joining the recipients into one string let an expectation match across recipient boundaries, so seeInLastEmailRecipient and seeInNthEmailRecipient passed for values no single recipient contains.
Test that nth positions follow created_at before the id tie-breaker, including the first and last position. Document that ...To addresses match by substring, as the existing ...To actions always have.
Jamesking56
left a comment
There was a problem hiding this comment.
Reviewed all changed PHP files and checked the README against the implementation. No actionable findings. The local unit suite passed with 31 tests and 60 assertions; all six CI test jobs passed.
|
Thanks @wazum |
mail-mime-parser returns null for a missing text or HTML part, which used to be passed to preg_match_all() and raised a deprecation notice. The null guard landed with captbaritone#130; this test keeps it from regressing. Resolves: captbaritone#138
|
Released in 4.1.0. |
Rebased and reworked version of #102, which was closed as stale.
@Jamesking56 as suggested in #102, here is a fresh PR against current
master.Resolves: #12, #98
What's new
seeInNthEmail($nth, $text),seeInNthEmailTo($nth, $address, $text),grabUrlsFromNthEmail($nth),seeNthEmailAttachmentCount($nth, $count),nthMessage($nth)andnthMessageTo($nth, $address). Positions start at 1 and count in the order the emails were received. The...Tovariants only count emails sent to that address.seeInLastEmailSender,seeInNthEmailSender,seeInLastEmailRecipientandseeInNthEmailRecipient. A recipient check passes when any one recipient matches.Email::getSender(). The sender is an optional last constructor argument, so existingnew Email(...)calls keep working.Fix
The sort tie-breaker from #13 compared
created_at . idas a string, which puts id 10 before id 9 when both arrive in the same second. That was mostly harmless for "last email" but breaks positional lookups. It now compares[created_at, id]tuples.Changes compared to #102
nthMessageFrom. It counted in the opposite direction fromnthMessageand no action used it.nthMessageTono longer counts an email twice when several of its recipients match.lastMessageFromunit test never ran because its name lacked thetestprefix. It is now a real test.Tests
Unit and acceptance suites pass on PHP 8.5 with the latest dependencies and on PHP 8.3 with
--prefer-lowest(Codeception 5.0.0, PHPUnit 9.6.0, mail-mime-parser 3.0.1): 84 tests, 132 assertions.