From 37efaeea757b266c37280df5b2d4857fa6967815 Mon Sep 17 00:00:00 2001 From: Noah Kantrowitz Date: Wed, 22 Jul 2015 14:10:09 -0700 Subject: Make the doc formatter actually show what version of a cookbook is being used. This has bugged me forever. It does change the API of the Formatter system in a ~non-back-compat way but I don't think I've actually seen any formatters outside of core Chef so how much do we want to worry about this? We could write a shim in `Chef::EventDispatcher::Dispatcher` to check the arity of the method if needed. --- lib/chef/cookbook/synchronizer.rb | 2 +- lib/chef/event_dispatch/base.rb | 4 ++-- lib/chef/formatters/doc.rb | 6 +++--- lib/chef/formatters/minimal.rb | 4 ++-- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/chef/cookbook/synchronizer.rb b/lib/chef/cookbook/synchronizer.rb index 1b96d0510b..4979aabfdf 100644 --- a/lib/chef/cookbook/synchronizer.rb +++ b/lib/chef/cookbook/synchronizer.rb @@ -131,7 +131,7 @@ class Chef files_remaining_by_cookbook[file.cookbook] -= 1 if files_remaining_by_cookbook[file.cookbook] == 0 - @events.synchronized_cookbook(file.cookbook.name) + @events.synchronized_cookbook(file.cookbook) end end diff --git a/lib/chef/event_dispatch/base.rb b/lib/chef/event_dispatch/base.rb index 50aee63450..7ed871145b 100644 --- a/lib/chef/event_dispatch/base.rb +++ b/lib/chef/event_dispatch/base.rb @@ -118,8 +118,8 @@ class Chef def cookbook_sync_start(cookbook_count) end - # Called when cookbook +cookbook_name+ has been sync'd - def synchronized_cookbook(cookbook_name) + # Called when cookbook +cookbook+ has been sync'd + def synchronized_cookbook(cookbook) end # Called when an individual file in a cookbook has been updated diff --git a/lib/chef/formatters/doc.rb b/lib/chef/formatters/doc.rb index 5ea9823d78..a40efb7298 100644 --- a/lib/chef/formatters/doc.rb +++ b/lib/chef/formatters/doc.rb @@ -133,9 +133,9 @@ class Chef indent end - # Called when cookbook +cookbook_name+ has been sync'd - def synchronized_cookbook(cookbook_name) - puts_line "- #{cookbook_name}" + # Called when cookbook +cookbook+ has been sync'd + def synchronized_cookbook(cookbook) + puts_line "- #{cookbook.name} (#{cookbook.version})" end # Called when an individual file in a cookbook has been updated diff --git a/lib/chef/formatters/minimal.rb b/lib/chef/formatters/minimal.rb index a189cc67eb..922a228f6e 100644 --- a/lib/chef/formatters/minimal.rb +++ b/lib/chef/formatters/minimal.rb @@ -109,8 +109,8 @@ class Chef puts "Synchronizing cookbooks" end - # Called when cookbook +cookbook_name+ has been sync'd - def synchronized_cookbook(cookbook_name) + # Called when cookbook +cookbook+ has been sync'd + def synchronized_cookbook(cookbook) print "." end -- cgit v1.2.1 From 85d3dfb6a361c774b6a1e4d4e437b836d5dd19cc Mon Sep 17 00:00:00 2001 From: Noah Kantrowitz Date: Fri, 31 Jul 2015 14:43:53 -0700 Subject: Revert back to the first argument being cookbook_name. --- lib/chef/cookbook/synchronizer.rb | 2 +- lib/chef/event_dispatch/base.rb | 2 +- lib/chef/formatters/doc.rb | 2 +- lib/chef/formatters/minimal.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/chef/cookbook/synchronizer.rb b/lib/chef/cookbook/synchronizer.rb index 4979aabfdf..fc8e739d73 100644 --- a/lib/chef/cookbook/synchronizer.rb +++ b/lib/chef/cookbook/synchronizer.rb @@ -131,7 +131,7 @@ class Chef files_remaining_by_cookbook[file.cookbook] -= 1 if files_remaining_by_cookbook[file.cookbook] == 0 - @events.synchronized_cookbook(file.cookbook) + @events.synchronized_cookbook(file.cookbook.name, file.cookbook) end end diff --git a/lib/chef/event_dispatch/base.rb b/lib/chef/event_dispatch/base.rb index 7ed871145b..0ae5101029 100644 --- a/lib/chef/event_dispatch/base.rb +++ b/lib/chef/event_dispatch/base.rb @@ -119,7 +119,7 @@ class Chef end # Called when cookbook +cookbook+ has been sync'd - def synchronized_cookbook(cookbook) + def synchronized_cookbook(cookbook_name, cookbook) end # Called when an individual file in a cookbook has been updated diff --git a/lib/chef/formatters/doc.rb b/lib/chef/formatters/doc.rb index a40efb7298..a5d7e210c5 100644 --- a/lib/chef/formatters/doc.rb +++ b/lib/chef/formatters/doc.rb @@ -134,7 +134,7 @@ class Chef end # Called when cookbook +cookbook+ has been sync'd - def synchronized_cookbook(cookbook) + def synchronized_cookbook(cookbook_name, cookbook) puts_line "- #{cookbook.name} (#{cookbook.version})" end diff --git a/lib/chef/formatters/minimal.rb b/lib/chef/formatters/minimal.rb index 922a228f6e..3862951f76 100644 --- a/lib/chef/formatters/minimal.rb +++ b/lib/chef/formatters/minimal.rb @@ -110,7 +110,7 @@ class Chef end # Called when cookbook +cookbook+ has been sync'd - def synchronized_cookbook(cookbook) + def synchronized_cookbook(cookbook_name, cookbook) print "." end -- cgit v1.2.1 From 63711b9da81c9db8466a2c45670f8d94cfdaac68 Mon Sep 17 00:00:00 2001 From: Noah Kantrowitz Date: Fri, 31 Jul 2015 14:45:34 -0700 Subject: Support forwards compatibility for event sinks. We can add new arguments and events without breaking compat as long as the new arguments are added at the end. The new args will be silently trimmed for sending to older event sinks, and new event types will be ignored. --- lib/chef/event_dispatch/dispatcher.rb | 12 +++++++++++- spec/unit/event_dispatch/dispatcher_spec.rb | 23 +++++++++++++++++++++-- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/lib/chef/event_dispatch/dispatcher.rb b/lib/chef/event_dispatch/dispatcher.rb index 370f8c51b4..9e17d78507 100644 --- a/lib/chef/event_dispatch/dispatcher.rb +++ b/lib/chef/event_dispatch/dispatcher.rb @@ -28,7 +28,17 @@ class Chef # Define a method that will be forwarded to all def self.def_forwarding_method(method_name) define_method(method_name) do |*args| - @subscribers.each { |s| s.send(method_name, *args) } + @subscribers.each do |s| + # Skip new/unsupported event names. + if s.respond_to?(method_name) + mth = s.method(method_name) + # Anything with a *args is arity -1, so use all arguments. + arity = mth.arity < 0 ? args.length : mth.arity + # Trim arguments to match what the subscriber expects to allow + # adding new arguments without breaking compat. + mth.call(*args.take(arity)) + end + end end end diff --git a/spec/unit/event_dispatch/dispatcher_spec.rb b/spec/unit/event_dispatch/dispatcher_spec.rb index 7e43b1933f..1014feea89 100644 --- a/spec/unit/event_dispatch/dispatcher_spec.rb +++ b/spec/unit/event_dispatch/dispatcher_spec.rb @@ -47,14 +47,33 @@ describe Chef::EventDispatch::Dispatcher do expect(event_sink).to receive(:run_start).with("12.4.0") dispatcher.run_start("12.4.0") - expect(event_sink).to receive(:synchronized_cookbook).with("apache2") - dispatcher.synchronized_cookbook("apache2") + cookbook_version = double("cookbook_version") + expect(event_sink).to receive(:synchronized_cookbook).with("apache2", cookbook_version) + dispatcher.synchronized_cookbook("apache2", cookbook_version) exception = StandardError.new("foo") expect(event_sink).to receive(:recipe_file_load_failed).with("/path/to/file.rb", exception) dispatcher.recipe_file_load_failed("/path/to/file.rb", exception) end + context "when an event sink has fewer arguments for an event" do + # Can't use a double because they don't report arity correctly. + let(:event_sink) do + Class.new(Chef::EventDispatch::Base) do + attr_reader :synchronized_cookbook_args + def synchronized_cookbook(cookbook_name) + @synchronized_cookbook_args = [cookbook_name] + end + end.new + end + + it "trims the arugment list" do + cookbook_version = double("cookbook_version") + dispatcher.synchronized_cookbook("apache2", cookbook_version) + expect(event_sink.synchronized_cookbook_args).to eq ["apache2"] + end + end + end end -- cgit v1.2.1 From abf85c2718ecc8cc70aa50d0c1e9560b40e7953a Mon Sep 17 00:00:00 2001 From: Noah Kantrowitz Date: Fri, 31 Jul 2015 14:45:42 -0700 Subject: Add a test for the new output. --- spec/unit/formatters/doc_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/spec/unit/formatters/doc_spec.rb b/spec/unit/formatters/doc_spec.rb index d018207f49..eb98f5abd3 100644 --- a/spec/unit/formatters/doc_spec.rb +++ b/spec/unit/formatters/doc_spec.rb @@ -43,4 +43,10 @@ describe Chef::Formatters::Base do expect(out.string).to include("Using policy 'jenkins' at revision '613f803bdd035d574df7fa6da525b38df45a74ca82b38b79655efed8a189e073'") end + it "prints cookbook name and version" do + cookbook_version = double(name: "apache2", version: "1.2.3") + formatter.synchronized_cookbook("apache2", cookbook_version) + expect(out.string).to include("- apache2 (1.2.3") + end + end -- cgit v1.2.1