Fix cask_sparkle_min_os audit options handling - #24039
Merged
Merged
Conversation
The `cask_sparkle_min_os` audit calls livecheck's `page_content` method directly but it doesn't pass the `livecheck` block options, so the audit won't work as expected in that scenario. For example, we recently hit the user agent deprecation logic in a version bump PR for the `izip` cask but that `livecheck` block contains a `user_agent: :browser` option, so that shouldn't be possible. I traced it back to this specific audit and confirmed that passing the `livecheck` block `options` fixes the issue. I checked other usage of `page_content` and `page_headers` and existing calls pass `options`, as we would expect. Besides that, I've added a guard to ensure that `livecheck.url` is set, as this check relies on a `livecheck` block URL being present. This is a relatively safe assumption but better safe than sorry.
This adds tests to expand coverage for the `cask_sparkle_min_os` audit to 100% for lines and branches. To achieve this without using `#send`, it was necessary to make the method public (following recent guidance around this). The only functional change in the method is to use `#fetch` in the `normalize_min_os` argument, as we're already guarding against `items.blank?`, so we couldn't exercise the `nil` path for `items[0]`.
samford
force-pushed
the
fix-cask_sparkle_min_os-options-handling
branch
from
September 20, 2026 01:13
82ce54e to
fdbff29
Compare
krehel
approved these changes
Sep 20, 2026
nandahkrishna
approved these changes
Sep 20, 2026
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.
brew benchmarkresults.brewcommands to reproduce the bug?brew lgtm(style, typechecking and tests) locally?I used Claude Code (Opus 5, high) to identify the source of the issue. I manually implemented a fix (confirming it with before/after runs of
brew audit --online izip) and had Claude Code review the changes. Afterward, I used Claude Code to implement tests to bring coverage for the method up to 100% (for lines and branches). I reviewed the tests, cleaned them up, and confirmed that they achieved the desired coverage by targeting the various states that I had identified.The
cask_sparkle_min_osaudit calls livecheck'spage_contentmethod directly but it doesn't pass thelivecheckblock options, so the audit won't work as expected in that scenario. For example, we recently hit the user agent deprecation logic in a version bump PR for theizipcask (Homebrew/homebrew-cask#288419) but thatlivecheckblock contains auser_agent: :browseroption, so that shouldn't be possible. I traced it back to this specific audit and confirmed that passing thelivecheckblockoptionsfixes the issue. I checked other usage ofpage_contentandpage_headersand existing calls passoptions, as we would expect.Besides that, I've added a guard to ensure that
livecheck.urlis set, as this check relies on alivecheckblock URL being present. This is a relatively safe assumption but better safe than sorry.This also adds tests to expand coverage for the
cask_sparkle_min_osaudit to 100% for lines and branches. To achieve this without using#send, it was necessary to make the method public (following recent guidance around this). The only functional change in the method is to use#fetchin thenormalize_min_osargument, as we're already guarding againstitems.blank?, so we couldn't exercise thenilpath foritems[0].This bug can be replicated by running
brew audit --online izip, which fails on themainbranch (with the user agent deprecation error) and passes on this PR branch.