diff options
author | The Bundler Bot <bot@bundler.io> | 2018-06-02 16:32:22 +0000 |
---|---|---|
committer | Colby Swandale <me@colby.fyi> | 2018-10-05 16:15:43 +1000 |
commit | 019b88d005f41deaf4cb453304591582d7de7b9c (patch) | |
tree | c090d7dd27d420df0ef7b710e0e4befd2c9c39a0 | |
parent | 1e60abf649ddc343a21c62c243047f5eb2cd5a59 (diff) | |
download | bundler-019b88d005f41deaf4cb453304591582d7de7b9c.tar.gz |
Auto merge of #6548 - bundler:jules2689/registered-events, r=colby-swandale
Make all plugin events registered to make documenting them easier
# What?
Every event in #hook and #add_hook will check if the event is registered in
Bundler::Plugin::Events. This allows for easy tracking of what's calling events,
and allow documentation to easily point to a single location.
It also makes testing easier as events are predicatable and accessible via constants
Thanks so much for the contribution!
To make reviewing this PR a bit easier, please fill out answers to the following questions.
---
### What was the end-user problem that led to this PR?
There wasn't really a problem, but this makes adding events easier
### What was your diagnosis of the problem?
Events are not tracked or documented well, this makes them documented well
### What is your fix for the problem, implemented in this PR?
Add an Events registration that hooks test for
### Why did you choose this fix out of the possible options?
Constants make things easily accessible via code, the hash makes it easy to check (with `O(1)` access!) for an event based on value as well.
(cherry picked from commit 3d829081f92b15cbb30cfb17eb5f87dee4a91b6e)
-rw-r--r-- | lib/bundler/installer.rb | 2 | ||||
-rw-r--r-- | lib/bundler/plugin.rb | 7 | ||||
-rw-r--r-- | lib/bundler/plugin/events.rb | 39 | ||||
-rw-r--r-- | spec/bundler/plugin/events_spec.rb | 18 | ||||
-rw-r--r-- | spec/bundler/plugin_spec.rb | 40 |
5 files changed, 94 insertions, 12 deletions
diff --git a/lib/bundler/installer.rb b/lib/bundler/installer.rb index 4956fad2ea..203c83fef3 100644 --- a/lib/bundler/installer.rb +++ b/lib/bundler/installer.rb @@ -21,7 +21,7 @@ module Bundler # For more information see the #run method on this class. def self.install(root, definition, options = {}) installer = new(root, definition) - Plugin.hook("before-install-all", definition.dependencies) + Plugin.hook(Plugin::Events::GEM_BEFORE_INSTALL_ALL, definition.dependencies) installer.run(options) installer end diff --git a/lib/bundler/plugin.rb b/lib/bundler/plugin.rb index 3f23fee2e0..0aa13ab784 100644 --- a/lib/bundler/plugin.rb +++ b/lib/bundler/plugin.rb @@ -5,6 +5,7 @@ require "bundler/plugin/api" module Bundler module Plugin autoload :DSL, "bundler/plugin/dsl" + autoload :Events, "bundler/plugin/events" autoload :Index, "bundler/plugin/index" autoload :Installer, "bundler/plugin/installer" autoload :SourceList, "bundler/plugin/source_list" @@ -155,6 +156,9 @@ module Bundler # To be called via the API to register a hooks and corresponding block that # will be called to handle the hook def add_hook(event, &block) + unless Events.defined_event?(event) + raise ArgumentError, "Event '#{event}' not defined in Bundler::Plugin::Events" + end @hooks_by_event[event.to_s] << block end @@ -166,6 +170,9 @@ module Bundler # @param [String] event def hook(event, *args, &arg_blk) return unless Bundler.feature_flag.plugins? + unless Events.defined_event?(event) + raise ArgumentError, "Event '#{event}' not defined in Bundler::Plugin::Events" + end plugins = index.hook_plugins(event) return unless plugins.any? diff --git a/lib/bundler/plugin/events.rb b/lib/bundler/plugin/events.rb new file mode 100644 index 0000000000..26bd59e5cf --- /dev/null +++ b/lib/bundler/plugin/events.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module Bundler + module Plugin + module Events + def self.define(const, event) + const = const.to_sym.freeze + if const_defined?(const) && const_get(const) != event + raise ArgumentError, "Attempting to reassign #{const} to a different value" + end + const_set(const, event) unless const_defined?(const) + @events ||= {} + @events[event] = const + end + private_class_method :define + + def self.reset + @events.each_value do |const| + remove_const(const) + end + @events = nil + end + private_class_method :reset + + # Check if an event has been defined + # @param event [String] An event to check + # @return [Boolean] A boolean indicating if the event has been defined + def self.defined_event?(event) + @events ||= {} + @events.key?(event) + end + + # @!parse + # # A hook called before any gems install + # GEM_BEFORE_INSTALL_ALL = "before-install-all" + define :GEM_BEFORE_INSTALL_ALL, "before-install-all" + end + end +end diff --git a/spec/bundler/plugin/events_spec.rb b/spec/bundler/plugin/events_spec.rb new file mode 100644 index 0000000000..b09e915682 --- /dev/null +++ b/spec/bundler/plugin/events_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +RSpec.describe Bundler::Plugin::Events do + context "plugin events" do + describe "#define" do + it "raises when redefining a constant" do + expect do + Bundler::Plugin::Events.send(:define, :GEM_BEFORE_INSTALL_ALL, "another-value") + end.to raise_error(ArgumentError) + end + + it "can define a new constant" do + Bundler::Plugin::Events.send(:define, :NEW_CONSTANT, "value") + expect(Bundler::Plugin::Events::NEW_CONSTANT).to eq("value") + end + end + end +end diff --git a/spec/bundler/plugin_spec.rb b/spec/bundler/plugin_spec.rb index 9796b580a3..9266fad1eb 100644 --- a/spec/bundler/plugin_spec.rb +++ b/spec/bundler/plugin_spec.rb @@ -228,6 +228,16 @@ RSpec.describe Bundler::Plugin do end end + describe "#add_hook" do + it "raises an ArgumentError on an unregistered event" do + ran = false + expect do + Plugin.add_hook("unregistered-hook") { ran = true } + end.to raise_error(ArgumentError) + expect(ran).to be(false) + end + end + describe "#hook" do before do path = lib_path("foo-plugin") @@ -235,7 +245,13 @@ RSpec.describe Bundler::Plugin do s.write "plugins.rb", code end - allow(index).to receive(:hook_plugins).with(event). + Bundler::Plugin::Events.send(:reset) + Bundler::Plugin::Events.send(:define, :EVENT_1, "event-1") + Bundler::Plugin::Events.send(:define, :EVENT_2, "event-2") + + allow(index).to receive(:hook_plugins).with(Bundler::Plugin::Events::EVENT_1). + and_return(["foo-plugin"]) + allow(index).to receive(:hook_plugins).with(Bundler::Plugin::Events::EVENT_2). and_return(["foo-plugin"]) allow(index).to receive(:plugin_path).with("foo-plugin").and_return(path) allow(index).to receive(:load_paths).with("foo-plugin").and_return([]) @@ -245,11 +261,15 @@ RSpec.describe Bundler::Plugin do Bundler::Plugin::API.hook("event-1") { puts "hook for event 1" } RUBY - let(:event) { "event-1" } + it "raises an ArgumentError on an unregistered event" do + expect do + Plugin.hook("unregistered-hook") + end.to raise_error(ArgumentError) + end it "executes the hook" do out = capture(:stdout) do - Plugin.hook("event-1") + Plugin.hook(Bundler::Plugin::Events::EVENT_1) end.strip expect(out).to eq("hook for event 1") @@ -257,17 +277,15 @@ RSpec.describe Bundler::Plugin do context "single plugin declaring more than one hook" do let(:code) { <<-RUBY } - Bundler::Plugin::API.hook("event-1") {} - Bundler::Plugin::API.hook("event-2") {} + Bundler::Plugin::API.hook(Bundler::Plugin::Events::EVENT_1) {} + Bundler::Plugin::API.hook(Bundler::Plugin::Events::EVENT_2) {} puts "loaded" RUBY - let(:event) { /event-1|event-2/ } - it "evals plugins.rb once" do out = capture(:stdout) do - Plugin.hook("event-1") - Plugin.hook("event-2") + Plugin.hook(Bundler::Plugin::Events::EVENT_1) + Plugin.hook(Bundler::Plugin::Events::EVENT_2) end.strip expect(out).to eq("loaded") @@ -276,12 +294,12 @@ RSpec.describe Bundler::Plugin do context "a block is passed" do let(:code) { <<-RUBY } - Bundler::Plugin::API.hook("#{event}") { |&blk| blk.call } + Bundler::Plugin::API.hook(Bundler::Plugin::Events::EVENT_1) { |&blk| blk.call } RUBY it "is passed to the hook" do out = capture(:stdout) do - Plugin.hook("event-1") { puts "win" } + Plugin.hook(Bundler::Plugin::Events::EVENT_1) { puts "win" } end.strip expect(out).to eq("win") |