Do not report notifications from guard skipped resources - #55
Open
tas50 wants to merge 1 commit into
Open
Conversation
Notifications stay declared on a resource even when a guard stopped its action from running, and the notify matcher read that declaration directly. A resource whose only_if never passed was therefore reported as notifying, even though ChefSpec correctly reported that its action was skipped and that the notified resource never ran. Tracking this needs care because Chef's ConditionalActionNotNothing makes should_skip? true for an :nothing action as well, so a resource that is notified and then notifies onward looks identical to a guard skipped one. Record genuine guard skips separately and only suppress notifications when the resource ran nothing and a guard refused it. Fixes chefspec#863 Fixes chefspec#751 Signed-off-by: Tim Smith <tsmith84@proton.me>
tas50
force-pushed
the
fix/guarded-notifications
branch
from
August 22, 2026 00:09
03d3f55 to
9c27f93
Compare
Author
|
CI on this PR is red for an unrelated reason: main's bundle cannot be installed on Linux, which breaks every job before any test runs. That is fixed separately in #56, which is green across Ruby 3.1 to 3.4. This PR should go green once #56 lands and this branch is rebased. Locally, against a working bundle, this branch passes |
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.
Fixes chefspec#863
Fixes chefspec#751
Problem
Notifications stay declared on a resource even when a guard stopped its action from running, and the notify matcher reads that declaration directly. So a resource whose
only_ifnever passed is still reported as notifying:This is an internal contradiction rather than a design tradeoff. In the same run ChefSpec correctly reports that the action was skipped and that the notified resource never ran:
Fix
The obvious fix, suppressing notifications when
performed_actionsis empty, is wrong. Chef'sConditionalActionNotNothingmakesshould_skip?true foraction :nothingtoo, so a guard skipped resource and a notifiedaction :nothingresource look identical. That naive version breaks legitimate chained notifications and fails the existingnotifications::chainedandsubscribesexamples.Instead, record genuine guard skips separately in
run_actionand only suppress notifications when the resource ran nothing and a guard refused it.Compatibility
This is the one behavior change in the set. A test that asserts a guarded resource notifies passes today and will start failing. That assertion was asserting something Chef does not do, but it is still visible breakage, so it is worth a changelog note.
Testing
Adds a
notifications::guardedacceptance example covering a guarded resource and an unguarded one for comparison.14 examples, 0 failures14 examples, 1 failurenotifications,subscribesandguardsexamples all pass197 examples, 0 failures