Skip to content

Add skip_all option for checks and check collections - #28

Open
rmm5t wants to merge 1 commit into
mainfrom
rmm/skip-all-option
Open

Add skip_all option for checks and check collections#28
rmm5t wants to merge 1 commit into
mainfrom
rmm/skip-all-option

Conversation

@rmm5t

@rmm5t rmm5t commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds a skip_all: true registration option for checks that should remain directly accessible without running as part of /okcomputer/all.

This also enables independently addressable groups of checks:

versions = OkComputer::CheckCollection.new("Versions")

OkComputer::Registry.register "versions", versions, skip_all: true
OkComputer::Registry.register "ruby_version", OkComputer::RubyVersionCheck.new, "versions"
OkComputer::Registry.register "app_version", OkComputer::AppVersionCheck.new, "versions"

The version checks are available through /okcomputer/versions and their individual endpoints, but are omitted from /okcomputer/all.

Implementation

  • Adds a skip_all attribute to checks and check collections.
  • Configures the default collection to omit entries marked skip_all.
  • Keeps skipped entries available through registry lookup.
  • Preserves normal behavior for named collections.
  • Supports excluding individual checks as well as entire collections.
  • Documents grouped checks and the new registration option.

Testing

  • Added coverage for execution, lookup, text output, JSON output, and success status.
  • Added coverage for skipped individual checks and collections.
  • Full suite passes with 461 examples, 0 failures.

/cc @dgarwood

@rmm5t rmm5t self-assigned this Sep 8, 2026
@rmm5t
rmm5t requested a review from emmahsax September 8, 2026 19:30
Comment on lines +151 to 157
it "omits checks registered with skip_all" do
foocheck.skip_all = true
default_collection.register(:foo, foocheck)
default_collection.register(:bar, barcheck)
allow(barcheck).to receive(:to_text) { "bar" }
expect(default_collection.to_text).to eq("foo collection name\n\s\sbar")
end

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is it possible that this passes because we have only the one allow? same question for the other tests like this. basically, what happens with this test if we include an allow(foocheck).to receive(:to_text) {"foo"}? If this is skipped, I'd expect that we get the expected output on line 156 and this test would pass.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. The test currently fails if the skipped check is included because its real output adds another line, but that relies unnecessarily on Check#to_text. I’ll stub both checks and add a negative expectation for the skipped check. I’ll make the equivalent assertions for JSON and success status as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

@rmm5t
rmm5t force-pushed the rmm/skip-all-option branch from db9278f to a5d5102 Compare September 8, 2026 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants