Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.markdown
Original file line number Diff line number Diff line change
@@ -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
Expand Down
26 changes: 26 additions & 0 deletions README.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion lib/ok_computer/check.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
18 changes: 13 additions & 5 deletions lib/ok_computer/check_collection.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -37,7 +40,7 @@ def [](key)
#
# Returns an Array of the collection's values
def checks
collection.values
included_collection.values
end

def <=>(check)
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand Down
23 changes: 20 additions & 3 deletions lib/ok_computer/registry.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,16 +31,33 @@ 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
#
# 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
Expand Down
69 changes: 61 additions & 8 deletions spec/ok_computer/check_collection_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Comment on lines +151 to 159

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.

end

Expand All @@ -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
Expand All @@ -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
31 changes: 31 additions & 0 deletions spec/ok_computer/registry_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down