Skip to content

Queue fake: better article, keep a custom Mail queue - #17

Merged
necessarylion merged 1 commit into
mainfrom
chore/review-nits
Nov 2, 2026
Merged

necessarylion merged 1 commit into
mainfrom
chore/review-nits

Conversation

@mydoxie

@mydoxie mydoxie commented Nov 2, 2026

Copy link
Copy Markdown
Contributor

Problem 1: the fake looked only at the first letter, so UserJob gave 'an UserJob'.
Fix: a vowel gives 'an', except names that start with Eu or U + consonant + vowel (User, Uni). Those give 'a'. Tests added.

Problem 2: Queue.restore() did not put back a queue set with Mail.useQueue() before Queue.fake().
Fix: fake() saves the current mail queue and restore() puts it back. Test added. Changelog lines updated.

@mydoxie

mydoxie commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Bot review

I ran dart analyze --fatal-infos, dart format check and dart test test/queue_fake_test.dart in a separate worktree with sibling clones. All clean, 11 tests pass.

Blocking

  • None. restore() puts back the right queue for the normal order (Mail.useQueue(x), then Queue.fake(), then restore()). A second fake() keeps the first saved queue. restore() clears the saved value, so nothing leaks to later tests. No public API change.

Non-blocking

  • The _youSound rule makes a wrong "a" for names that start with Un + vowel, like UnusedJob, UnauthorizedJob, UnarchiveJob (the u sounds like "uh", so "an" was right before). Fix idea: do not match when the name starts with Un + vowel, or only match a short list (Use, User, Uni, Util, Eu).
  • If Mail.fake() is on when Queue.fake() runs, _currentMailQueue() returns null (it hides the fake), so a custom Mail.useQueue() queue set before is lost on restore(). Rare. Worth one line in the doc comment.
  • restore() with no fake() before still overwrites a custom queue with null or QueueMailQueue. This is old behaviour, not new.
  • The doc comment of _a (a SendWelcomeEmail, ...) now sits on the _youSound field, so _a has no doc comment. Move _youSound above that comment.
  • The new test does not reset Mail.useQueue(null) if an expect fails before the last line. Use addTearDown(() => Mail.useQueue(null)).
  • No version: bump. Changelog lines are under Unreleased and plain.

@necessarylion
necessarylion merged commit 6b049ea into main Nov 2, 2026
1 check passed
@necessarylion
necessarylion deleted the chore/review-nits branch November 2, 2026 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants