28: Prove the notice was there before asserting it is gone - #42
Open
nikolaystrikhar wants to merge 1 commit into
Open
28: Prove the notice was there before asserting it is gone#42nikolaystrikhar wants to merge 1 commit into
nikolaystrikhar wants to merge 1 commit into
Conversation
Three clear-then-assertFalse tests never established the row existed, so a store that wrote nothing passed them. The renderer's not-called assertion had its control in a sibling test. The replacement-store test overrode all(), put() and clear() but not option_name(), so a writer composing the name itself -- instead of delegating, which is the documented seam -- passed. Adds the multisite scope neither option had: the queue is written, a second site is created, and switch_to_blog() finds it still there. Swapping the store to get_option()/update_option() now fails the multisite leg, where before it was green on both. Also pins that the capability is checked before the store is read. The check guards the clearing as much as the drawing, and nothing said so.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What: the four
Noticestest files prove state exists before asserting it is gone, cover the store'soption_name()seam, pin the capability-before-read ordering, and add the multisite scope test the queue never had.Usage:
Why this way:
An
assertFalseafter a clear passes for a store that never wrote. Three tests had no in-test proof the row existed; one in the same file already did it correctly, so the shape was there to copy.The replacement-store double overrode everything except the method under test.
Notices\Writer::option_name()delegates to the store — that is the seam a host moving the queue relies on — but a writer composing the name fromConfig::get_option_name()instead passed the file.Network scope was green either way. No test in the suite called
switch_to_blog(), so swappingget_site_option()forget_option()broke nothing. It now creates a second site and reads the queue from it.The capability check guards the clearing, not just the drawing. Nothing asserted it runs before the store is read, so a user who may not see the queue could have destroyed it.