From a5d5102780d64f0c27d18ad4a8bfd210cbce0486 Mon Sep 17 00:00:00 2001 From: Ryan McGeary Date: Tue, 8 Sep 2026 13:26:35 -0600 Subject: [PATCH 1/3] Add skip_all registration option --- CHANGELOG.markdown | 2 + README.markdown | 26 +++++++++ lib/ok_computer/check.rb | 2 +- lib/ok_computer/check_collection.rb | 18 ++++-- lib/ok_computer/registry.rb | 23 +++++++- spec/ok_computer/check_collection_spec.rb | 69 ++++++++++++++++++++--- spec/ok_computer/registry_spec.rb | 31 ++++++++++ 7 files changed, 154 insertions(+), 17 deletions(-) diff --git a/CHANGELOG.markdown b/CHANGELOG.markdown index 27bf108c..d5aae2d2 100644 --- a/CHANGELOG.markdown +++ b/CHANGELOG.markdown @@ -1,4 +1,6 @@ #### Unreleased +* Add a `skip_all` registration option for checks and check collections that + should remain directly accessible without running at `/okcomputer/all` * Don't use a shared closure for each spawned thread > awilfox: https://github.com/okcomputer-ruby/okcomputer/pull/27 * ActionMailerCheck: Support :sendmail and :test diff --git a/README.markdown b/README.markdown index 6ea05b1b..a568bd35 100644 --- a/README.markdown +++ b/README.markdown @@ -182,6 +182,32 @@ end OkComputer::Registry.register "check_for_odds", MyCustomCheck.new ``` +### Grouping Checks + +Use a `CheckCollection` to expose several related checks from one endpoint. Register +the collection with `skip_all: true` when the group should not run as part of the +default `/okcomputer/all` endpoint: + +```ruby +# config/initializers/okcomputer.rb +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 group is available at `/okcomputer/versions` and `/okcomputer/versions.json`. +Its checks remain individually available, but neither the group nor its checks run +at `/okcomputer/all`. + +An individual check can also be omitted from `/okcomputer/all` while retaining its +own endpoint: + +```ruby +OkComputer::Registry.register "ruby_version", OkComputer::RubyVersionCheck.new, skip_all: true +``` + ### Registering Optional Checks Register an optional check like so: diff --git a/lib/ok_computer/check.rb b/lib/ok_computer/check.rb index 8980d934..43786d7d 100644 --- a/lib/ok_computer/check.rb +++ b/lib/ok_computer/check.rb @@ -3,7 +3,7 @@ module OkComputer class Check # to be set by Registry upon registration - attr_accessor :registrant_name + attr_accessor :registrant_name, :skip_all # nil by default, only set to true if the check deems itself failed attr_accessor :failure_occurred # nil by default, set by #check to control the output diff --git a/lib/ok_computer/check_collection.rb b/lib/ok_computer/check_collection.rb index 4073733c..fc9d37cd 100644 --- a/lib/ok_computer/check_collection.rb +++ b/lib/ok_computer/check_collection.rb @@ -1,13 +1,16 @@ module OkComputer class CheckCollection - attr_accessor :collection, :registrant_name, :display + attr_accessor :collection, :registrant_name, :display, :skip_all # Public: Initialize a new CheckCollection # # display - the display name for the Check Collection - def initialize(display) + # exclude_skipped_checks - whether checks marked skip_all should be omitted + def initialize(display, exclude_skipped_checks=false) self.display = display self.collection = {} + self.skip_all = false + @exclude_skipped_checks = exclude_skipped_checks end # Public: Run the collection's checks @@ -37,7 +40,7 @@ def [](key) # # Returns an Array of the collection's values def checks - collection.values + included_collection.values end def <=>(check) @@ -51,13 +54,13 @@ def <=>(check) alias_method :values, :checks def check_names - collection.keys + included_collection.keys end alias_method :keys, :check_names def sub_collections - checks.select{ |c| c.is_a?(CheckCollection)} + collection.values.select{ |c| c.is_a?(CheckCollection)} end def self_and_sub_collections @@ -108,6 +111,11 @@ def success? private + def included_collection + return collection unless @exclude_skipped_checks + collection.reject{ |_name, check| check.respond_to?(:skip_all) && check.skip_all } + end + def check_in_sequence checks.each(&:run) end diff --git a/lib/ok_computer/registry.rb b/lib/ok_computer/registry.rb index bbc75963..42bb3413 100644 --- a/lib/ok_computer/registry.rb +++ b/lib/ok_computer/registry.rb @@ -31,7 +31,7 @@ def self.all # # Returns @default_collection def self.default_collection - @default_collection ||= CheckCollection.new('Default Collection') + @default_collection ||= CheckCollection.new('Default Collection', true) end # Public: Register the given check with OkComputer @@ -39,8 +39,25 @@ def self.default_collection # check_name - The name of the check to retrieve # check_object - Instance of Checker to register # collection_name - The name of the check collection the check should be registered to - def self.register(check_name, check_object, collection_name=nil) - find_collection(collection_name).register(check_name, check_object) + # options - Set skip_all to true to omit the check from the default collection's results + def self.register(check_name, check_object, collection_name=nil, options={}) + if collection_name.is_a?(Hash) + options = collection_name + collection_name = nil + end + + if collection_name && options[:skip_all] + raise ArgumentError, "skip_all is only supported in the default collection" + end + + if !collection_name && check_object.respond_to?(:skip_all=) + check_object.skip_all = !!options[:skip_all] + elsif options[:skip_all] + raise ArgumentError, "skip_all requires a check that supports skip_all=" + end + + collection = find_collection(collection_name) + collection.register(check_name, check_object) end # Public: Remove the check of the given name being checked diff --git a/spec/ok_computer/check_collection_spec.rb b/spec/ok_computer/check_collection_spec.rb index e139337f..8bda43c9 100644 --- a/spec/ok_computer/check_collection_spec.rb +++ b/spec/ok_computer/check_collection_spec.rb @@ -2,16 +2,12 @@ module OkComputer describe CheckCollection do - let(:foocheck) { double(:check) } - let(:barcheck) { double(:check) } + let(:foocheck) { Check.new } + let(:barcheck) { Check.new } let(:registry) { {foo: foocheck, bar: barcheck} } - before do - allow(foocheck).to receive(:registrant_name=) - allow(barcheck).to receive(:registrant_name=) - end - subject { CheckCollection.new("foo collection name") } + let(:default_collection) { CheckCollection.new("foo collection name", true) } context ".new" do it "sets the display name of the check collection" do @@ -33,6 +29,15 @@ module OkComputer expect(barcheck).to receive(:run) subject.run end + + it "does not run checks registered with skip_all" do + foocheck.skip_all = true + default_collection.register(:foo, foocheck) + default_collection.register(:bar, barcheck) + expect(foocheck).not_to receive(:run) + expect(barcheck).to receive(:run) + default_collection.run + end end end end @@ -43,6 +48,19 @@ module OkComputer subject.register(:bar, barcheck) expect(subject.checks).to eq(registry.values) end + + it "omits checks registered with skip_all" do + foocheck.skip_all = true + default_collection.register(:foo, foocheck) + default_collection.register(:bar, barcheck) + expect(default_collection.checks).to eq([barcheck]) + end + + it "does not omit skipped checks from a named collection" do + foocheck.skip_all = true + subject.register(:foo, foocheck) + expect(subject.checks).to eq([foocheck]) + end end context "#register" do @@ -67,6 +85,12 @@ module OkComputer expect(subject.fetch(:foo)).to eq(foocheck) end + it "finds checks registered with skip_all" do + foocheck.skip_all = true + subject.register(:foo, foocheck) + expect(subject.fetch(:foo)).to eq(foocheck) + end + it "finds checks in a sub_collection" do sub_collection = CheckCollection.new("sub") subject.register("sub", sub_collection) @@ -121,7 +145,17 @@ module OkComputer subject.register(:bar, barcheck) allow(foocheck).to receive(:to_text) { "foo" } allow(barcheck).to receive(:to_text) { "bar" } - expect(subject.to_text).to eq("foo collection name\n\s\sfoo\n\s\sbar") + expect(subject.to_text).to eq("foo collection name\n\s\sbar\n\s\sfoo") + end + + it "omits checks registered with skip_all" do + foocheck.skip_all = true + default_collection.register(:foo, foocheck) + default_collection.register(:bar, barcheck) + allow(foocheck).to receive(:to_text) { "foo" } + allow(barcheck).to receive(:to_text) { "bar" } + expect(foocheck).not_to receive(:to_text) + expect(default_collection.to_text).to eq("foo collection name\n\s\sbar") end end @@ -134,6 +168,16 @@ module OkComputer combined_hash = JSON.parse(foocheck.to_json).merge(JSON.parse(barcheck.to_json)) expect(subject.to_json).to eq(combined_hash.to_json) end + + it "omits checks registered with skip_all" do + foocheck.skip_all = true + default_collection.register(:foo, foocheck) + default_collection.register(:bar, barcheck) + allow(foocheck).to receive(:to_json) { {"foo" => "foo result"}.to_json } + allow(barcheck).to receive(:to_json) { {"bar" => "bar result"}.to_json } + expect(foocheck).not_to receive(:to_json) + expect(default_collection.to_json).to eq({"bar" => "bar result"}.to_json) + end end context "#success?" do @@ -152,6 +196,15 @@ module OkComputer allow(barcheck).to receive(:success?) { false } expect(subject).not_to be_success end + + it "ignores failures from checks registered with skip_all" do + foocheck.skip_all = true + default_collection.register(:foo, foocheck) + default_collection.register(:bar, barcheck) + allow(barcheck).to receive(:success?) { true } + expect(foocheck).not_to receive(:success?) + expect(default_collection).to be_success + end end end end diff --git a/spec/ok_computer/registry_spec.rb b/spec/ok_computer/registry_spec.rb index 9e8b2208..219369b4 100644 --- a/spec/ok_computer/registry_spec.rb +++ b/spec/ok_computer/registry_spec.rb @@ -62,6 +62,20 @@ module OkComputer Registry.register(check_name, check_object) end + it "keeps a check fetchable when skip_all is true" do + skipped_check = Check.new + Registry.register(check_name, skipped_check, skip_all: true) + expect(Registry.fetch(check_name)).to eq(skipped_check) + expect(Registry.all.checks).not_to include(skipped_check) + end + + it "includes a skipped check when it is registered again without skip_all" do + skipped_check = Check.new + Registry.register(check_name, skipped_check, skip_all: true) + Registry.register(check_name, skipped_check) + expect(Registry.all.checks).to include(skipped_check) + end + it "throws a collection not found error if a collection with the given name is not found" do expect { Registry.register(check_name, check_object, "missing collection") }.to raise_error(Registry::CollectionNotFound) end @@ -73,6 +87,23 @@ module OkComputer expect(collection.fetch(check_name)).to eq(check_object) end + it "can omit a check collection and its checks from all" do + collection = CheckCollection.new('Versions') + Registry.register('versions', collection, skip_all: true) + Registry.register(check_name, check_object, 'versions') + + expect(Registry.fetch('versions')).to eq(collection) + expect(Registry.fetch(check_name)).to eq(check_object) + expect(Registry.all.checks).not_to include(collection) + end + + it "rejects skip_all when registering inside a check collection" do + Registry.register('test_collection', collection) + expect { + Registry.register(check_name, check_object, 'test_collection', skip_all: true) + }.to raise_error(ArgumentError, /default collection/) + end + it "gracefully handles checks defined with a combination of strings and symbols as their name" do Registry.register("foo", Check.new) Registry.register(:bar, Check.new) From 28b0dc676225807602bba15d9c015fed0d61c0b4 Mon Sep 17 00:00:00 2001 From: Ryan McGeary Date: Thu, 10 Sep 2026 09:41:08 -0600 Subject: [PATCH 2/3] Preserve skip_all when re-registering checks --- lib/ok_computer/registry.rb | 10 ++++++---- spec/ok_computer/registry_spec.rb | 19 ++++++++++++++++++- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/lib/ok_computer/registry.rb b/lib/ok_computer/registry.rb index 42bb3413..2fa61a21 100644 --- a/lib/ok_computer/registry.rb +++ b/lib/ok_computer/registry.rb @@ -50,10 +50,12 @@ def self.register(check_name, check_object, collection_name=nil, options={}) raise ArgumentError, "skip_all is only supported in the default collection" end - if !collection_name && check_object.respond_to?(:skip_all=) - check_object.skip_all = !!options[:skip_all] - elsif options[:skip_all] - raise ArgumentError, "skip_all requires a check that supports skip_all=" + if !collection_name && options.key?(:skip_all) + if check_object.respond_to?(:skip_all=) + check_object.skip_all = !!options[:skip_all] + elsif options[:skip_all] + raise ArgumentError, "skip_all requires a check that supports skip_all=" + end end collection = find_collection(collection_name) diff --git a/spec/ok_computer/registry_spec.rb b/spec/ok_computer/registry_spec.rb index 219369b4..9ecb7094 100644 --- a/spec/ok_computer/registry_spec.rb +++ b/spec/ok_computer/registry_spec.rb @@ -69,13 +69,30 @@ module OkComputer expect(Registry.all.checks).not_to include(skipped_check) end - it "includes a skipped check when it is registered again without skip_all" do + it "preserves skip_all when a check is registered again without the option" do skipped_check = Check.new Registry.register(check_name, skipped_check, skip_all: true) Registry.register(check_name, skipped_check) + expect(Registry.all.checks).not_to include(skipped_check) + end + + it "clears skip_all when a check is registered again with skip_all false" do + skipped_check = Check.new + Registry.register(check_name, skipped_check, skip_all: true) + Registry.register(check_name, skipped_check, skip_all: false) expect(Registry.all.checks).to include(skipped_check) end + it "preserves skip_all when making a check optional" do + skipped_check = Check.new + Registry.register(check_name, skipped_check, skip_all: true) + OkComputer.make_optional [check_name] + + optional_check = Registry.fetch(check_name) + expect(optional_check).to be_a(OkComputer::OptionalCheck) + expect(Registry.all.checks).not_to include(optional_check) + end + it "throws a collection not found error if a collection with the given name is not found" do expect { Registry.register(check_name, check_object, "missing collection") }.to raise_error(Registry::CollectionNotFound) end From 5b0c563bcfcc06b5bc9c661c97f94ca0b5cf4b3b Mon Sep 17 00:00:00 2001 From: Ryan McGeary Date: Thu, 10 Sep 2026 09:53:39 -0600 Subject: [PATCH 3/3] Isolate registry state between specs Fix flakey specs --- spec/ok_computer/configuration_spec.rb | 4 +--- spec/ok_computer/registry_spec.rb | 10 ++++------ spec/support/helpers.rb | 12 ++++++++++++ 3 files changed, 17 insertions(+), 9 deletions(-) diff --git a/spec/ok_computer/configuration_spec.rb b/spec/ok_computer/configuration_spec.rb index 6c9a1bae..4022787c 100644 --- a/spec/ok_computer/configuration_spec.rb +++ b/spec/ok_computer/configuration_spec.rb @@ -128,9 +128,7 @@ end around(:each) do |example| - existing = OkComputer::Registry.instance_variable_get(:@registry) - example.run - OkComputer::Registry.instance_variable_set(:@registry, existing) + with_clean_registry { example.run } end it "marks listed checks as optional" do diff --git a/spec/ok_computer/registry_spec.rb b/spec/ok_computer/registry_spec.rb index 9ecb7094..44268ee5 100644 --- a/spec/ok_computer/registry_spec.rb +++ b/spec/ok_computer/registry_spec.rb @@ -5,6 +5,10 @@ module OkComputer let(:check_object) { double(:first_checker, :registrant_name= => nil) } let(:collection) { CheckCollection.new('foo collection') } + around do |example| + with_clean_registry { example.run } + end + context ".all" do it "returns a CheckCollection with all of the registered checks" do expect(Registry.all).to be_instance_of(CheckCollection) @@ -29,12 +33,6 @@ module OkComputer let(:second_check_object) { double(:second_checker, :registrant_name= => nil) } let(:default_collection) { double } - after do - # Clear out registered checks to avoid leaking test doubles - Registry.instance_variable_defined?(:@default_collection) && - Registry.remove_instance_variable(:@default_collection) - end - it "assigns the given name to the check" do expect(check_object).to receive(:registrant_name=).with(check_name) Registry.register(check_name, check_object) diff --git a/spec/support/helpers.rb b/spec/support/helpers.rb index 06ea556f..fa46882c 100644 --- a/spec/support/helpers.rb +++ b/spec/support/helpers.rb @@ -12,4 +12,16 @@ def with_env(env) ENV.replace(original) end + + def with_clean_registry + registry = OkComputer::Registry + default_collection_defined = registry.instance_variable_defined?(:@default_collection) + default_collection = registry.instance_variable_get(:@default_collection) + registry.remove_instance_variable(:@default_collection) if default_collection_defined + + yield + ensure + registry.remove_instance_variable(:@default_collection) if registry.instance_variable_defined?(:@default_collection) + registry.instance_variable_set(:@default_collection, default_collection) if default_collection_defined + end end