From bdc4120cefb1f4fff870a217ce97fc4a876385b6 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Fri, 12 Jun 2015 16:02:28 +0200 Subject: [PATCH 01/47] drop Ruby 1.9 build in Travis --- .travis.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index f248fb91..f636367e 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,8 +1,7 @@ language: ruby bundler_args: --without development rvm: - - 1.9.3 - - 2.2.0 + - 2.2.1 - jruby - rbx-2 matrix: From 39aa8f573a5d177028adc2fe34025f01047bc1fc Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Fri, 12 Jun 2015 21:54:06 +0200 Subject: [PATCH 02/47] fix exception message in threaded Adapter --- lib/listen/adapter/base.rb | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/lib/listen/adapter/base.rb b/lib/listen/adapter/base.rb index 89c2f5b3..a90e30f8 100644 --- a/lib/listen/adapter/base.rb +++ b/lib/listen/adapter/base.rb @@ -58,7 +58,10 @@ def start begin _run rescue - _log_exception "run() in thread failed: %s:\n %s\n\ncalled from:\n %s", calling_stack + msg = "run() in thread failed: %s:\n"\ + " %s\n\ncalled from:\n %s", calling_stack + + _log_exception(msg) raise end end @@ -85,7 +88,12 @@ def _log(*args, &block) end def _log_exception(msg, caller_stack) - _log :error, format(msg, $ERROR_INFO, $ERROR_POSITION * "\n", caller_stack * "\n") + _log :error, format( + msg, + $ERROR_INFO, + $ERROR_POSITION * "\n", + caller_stack * "\n" + ) end def self._log(*args, &block) From 4df8c9e07ed2a3b5958532f90776872972cfe66c Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Fri, 12 Jun 2015 15:00:08 +0200 Subject: [PATCH 03/47] remove TCP functionality for Listen 3.x --- .travis.yml | 2 +- Gemfile | 1 - README.md | 44 +------- lib/listen.rb | 43 +------- lib/listen/adapter.rb | 2 - lib/listen/adapter/tcp.rb | 88 --------------- lib/listen/cli.rb | 8 -- lib/listen/listener.rb | 31 ------ lib/listen/tcp.rb | 8 -- lib/listen/tcp/broadcaster.rb | 79 -------------- lib/listen/tcp/message.rb | 50 --------- spec/acceptance/tcp_spec.rb | 136 ------------------------ spec/lib/listen/adapter/tcp_spec.rb | 127 ---------------------- spec/lib/listen/adapter_spec.rb | 5 - spec/lib/listen/tcp/broadcaster_spec.rb | 122 --------------------- spec/lib/listen/tcp/listener_spec.rb | 102 ------------------ spec/lib/listen/tcp/message_spec.rb | 136 ------------------------ spec/lib/listen_spec.rb | 29 +---- spec/spec_helper.rb | 1 - 19 files changed, 8 insertions(+), 1006 deletions(-) delete mode 100644 lib/listen/adapter/tcp.rb delete mode 100644 lib/listen/tcp.rb delete mode 100644 lib/listen/tcp/broadcaster.rb delete mode 100644 lib/listen/tcp/message.rb delete mode 100644 spec/acceptance/tcp_spec.rb delete mode 100644 spec/lib/listen/adapter/tcp_spec.rb delete mode 100644 spec/lib/listen/tcp/broadcaster_spec.rb delete mode 100644 spec/lib/listen/tcp/listener_spec.rb delete mode 100644 spec/lib/listen/tcp/message_spec.rb diff --git a/.travis.yml b/.travis.yml index f636367e..089fe540 100644 --- a/.travis.yml +++ b/.travis.yml @@ -17,5 +17,5 @@ os: - linux - osx env: - - LISTEN_TESTS_DEFAULT_LAG=0.8 LISTEN_TESTS_DEFAULT_TCP_LAG=1.2 + - LISTEN_TESTS_DEFAULT_LAG=0.8 sudo: false diff --git a/Gemfile b/Gemfile index c3f8a30a..1ccb6ca6 100644 --- a/Gemfile +++ b/Gemfile @@ -14,7 +14,6 @@ end group :test do gem 'celluloid', github: 'celluloid/celluloid', branch: '0-16-stable' - gem 'celluloid-io', '>= 0.15.0' gem 'rake' gem 'rspec', '~> 3.2' gem 'rspec-retry' diff --git a/README.md b/README.md index 6b46908e..79db5b2b 100644 --- a/README.md +++ b/README.md @@ -20,7 +20,6 @@ Make sure you know these few basic tricks: https://github.com/guard/listen/wiki/ * Detects file modification, addition and removal. * You can watch multiple directories. * Regexp-patterns for ignoring paths for more accuracy and speed -* Forwarding file events over TCP, [more info](#forwarding-file-events-over-tcp) below. * Increased change detection accuracy on OS X HFS and VFAT volumes. * Tested on MRI Ruby environments (1.9+ only) via [Travis CI](https://travis-ci.org/guard/listen), @@ -29,6 +28,8 @@ Please note that: - Specs suite on JRuby and Rubinius aren't reliable on Travis CI, but should work. - Windows and \*BSD adapter aren't continuously and automaticaly tested. +NOTE: TCP functionality has been moved to a separate gem (listen-server and listen-client) + ## Pending features / issues * symlinked directories aren't fully transparent yet: https://github.com/guard/listen/issues/279 @@ -247,47 +248,6 @@ Also, if the directories you're watching contain many files, make sure you're: When in doubt, LISTEN_GEM_DEBUGGING=2 can help discover the actual events and time they happened. -## Forwarding file events over TCP - -Listen is capable of forwarding file events over the network using a messaging protocol. This can be useful for virtualized development environments when file events are unavailable, as is the case with shared folders in VMs. - -[Vagrant](https://github.com/mitchellh/vagrant) uses Listen in it's rsync-auto mode to solve this issue. - -To broadcast events over TCP programmatically, use the `forward_to` option with an address - just a port or a hostname/port combination: - -```ruby -listener = Listen.to 'path/to/app', forward_to: '10.0.0.2:4000' do |modified, added, removed| - # After broadcasting the changes to any connected recipients, - # this block will still be called -end -listener.start -sleep -``` - -As a convenience, the `listen` script is supplied which listens to a directory and forwards the events to a network address - -```bash -listen -f "10.0.0.2:4000" # changes in current directory are sent as absolute paths -listen -r -f "10.0.0.2:4000" # changes in current directory are sent as relative paths -listen -v -d "/projects/my_project" -f "10.0.0.2:4000" # changes in given directory are also shown -``` - -*NOTE: if you are using a gem like `guard` and the paths on host and guest are not exactly the same, you'll generally want to use the `-r` option for relative paths* - -To connect to a broadcasting listener as a recipient, specify its address using `Listen.on`: - -```ruby -listener = Listen.on '10.0.0.2:4000' do |modified, added, removed| - # This block will be called -end -listener.start -sleep -``` - -### Security considerations - -Since file events potentially expose sensitive information, care must be taken when specifying the broadcaster address. It is recommended to **always** specify a hostname and make sure it is as specific as possible to reduce any undesirable eavesdropping. - ## Development * Documentation hosted at [RubyDoc](http://rubydoc.info/github/guard/listen/master/frames). diff --git a/lib/listen.rb b/lib/listen.rb index e4941471..4ad54048 100644 --- a/lib/listen.rb +++ b/lib/listen.rb @@ -8,9 +8,6 @@ class << self # Listens to file system modifications on a either single directory or # multiple directories. # - # When :forward_to is specified, this listener will broadcast modifications - # over TCP. - # # @param (see Listen::Listener#new) # # @yield [modified, added, removed] the changed files @@ -21,18 +18,12 @@ class << self # @return [Listen::Listener] the listener # def to(*args, &block) - Celluloid.boot unless Celluloid.running? - options = args.last.is_a?(Hash) ? args.last : {} - target = options.delete(:forward_to) - args = ([target, :broadcaster] + args) if target - _add_listener(*args, &block) + @listeners ||= [] + Listener.new(*args, &block).tap do |listener| + @listeners << listener + end end - # Stop all listeners & Celluloid - # - # Use it for testing purpose or when you are sure that Celluloid could be - # ended. - # # This is used by the `listen` binary to handle Ctrl-C # def stop @@ -45,32 +36,6 @@ def stop listener.stop end @listeners = nil - - Celluloid.shutdown - end - - # Listens to file system modifications broadcast over TCP. - # - # @param [String/Fixnum] target to listen on (hostname:port or port) - # - # @yield [modified, added, removed] the changed files - # @yieldparam [Array] modified the list of modified files - # @yieldparam [Array] added the list of added files - # @yieldparam [Array] removed the list of removed files - # - # @return [Listen::Listener] the listener - # - def on(target, *args, &block) - _add_listener(target, :recipient, *args, &block) - end - - private - - def _add_listener(*args, &block) - @listeners ||= [] - Listener.new(*args, &block).tap do |listener| - @listeners << listener - end end end end diff --git a/lib/listen/adapter.rb b/lib/listen/adapter.rb index 058b8f50..eaa319fc 100644 --- a/lib/listen/adapter.rb +++ b/lib/listen/adapter.rb @@ -12,8 +12,6 @@ module Adapter 'Learn more at https://github.com/guard/listen#listen-adapters.' def self.select(options = {}) - _log :debug, 'Adapter: considering TCP ...' - return TCP if options[:force_tcp] _log :debug, 'Adapter: considering polling ...' return Polling if options[:force_polling] _log :debug, 'Adapter: considering optimized backend...' diff --git a/lib/listen/adapter/tcp.rb b/lib/listen/adapter/tcp.rb deleted file mode 100644 index 752bd39a..00000000 --- a/lib/listen/adapter/tcp.rb +++ /dev/null @@ -1,88 +0,0 @@ -require 'celluloid/io' - -require 'listen/tcp/message' - -module Listen - module Adapter - # Adapter to receive file system modifications over TCP - class TCP < Base - OS_REGEXP = // # match any - - include Celluloid::IO - finalizer :finalize - - DEFAULTS = { - host: 'localhost', - port: '4000' - } - - attr_reader :buffer, :socket - - # Initializes and starts a Celluloid::IO-powered TCP-recipient - def start - attempts ||= 3 - _log :info, "TCP: opening socket #{options.host}:#{options.port}" - @socket = TCPSocket.new(options.host, options.port) - @buffer = '' - async.run - rescue Celluloid::Task::TerminatedError - _log :debug, "TCP adapter was terminated: #{$ERROR_INFO.inspect}" - rescue Errno::ECONNREFUSED - sleep 1 - attempts -= 1 - _log :warn, "TCP.start: #{$ERROR_INFO.inspect}" - retry if attempts > 0 - _log :error, format('TCP.start: %s:%s', $ERROR_INFO.inspect, - $ERROR_POSITION * "\n") - raise - rescue - _log :error, format('TCP.start: %s:%s', $ERROR_INFO.inspect, - $ERROR_POSITION * "\n") - raise - end - - # Cleans up buffer and socket - def finalize - @buffer = nil - return unless @socket - - @socket.close - @socket = nil - end - - # Number of bytes to receive at a time - RECEIVE_WINDOW = 1024 - - # Continuously receive and asynchronously handle data - def run - while (data = @socket.recv(RECEIVE_WINDOW)) - async.handle_data(data) - end - end - - # Buffers incoming data and handles messages accordingly - def handle_data(data) - @buffer << data - while (message = Listen::TCP::Message.from_buffer(@buffer)) - handle_message(message) - end - rescue - _log :error, format('TCP.handle_data crashed: %s:%s', $ERROR_INFO, - $ERROR_POSITION * "\n") - raise - end - - # Handles incoming message by notifying of path changes - def handle_message(message) - type, change, dir, path, _ = message.object - _log(:debug) { "TCP message: #{[type, change, dir, path].inspect}" } - - _queue_change(type.to_sym, Pathname(dir), path, change: change.to_sym) - end - - def self.local_fs? - false - end - end - end -end diff --git a/lib/listen/cli.rb b/lib/listen/cli.rb index 06bdd2c4..2e677dc4 100644 --- a/lib/listen/cli.rb +++ b/lib/listen/cli.rb @@ -14,12 +14,6 @@ class CLI < Thor aliases: '-v', banner: 'Verbose' - class_option :forward, - type: :string, - default: '127.0.0.1:4000', - aliases: '-f', - banner: 'The address to forward filesystem events' - class_option :directory, type: :array, default: '.', @@ -48,7 +42,6 @@ def initialize(options) def start logger.info 'Starting listen...' - address = @options[:forward] directory = @options[:directory] relative = @options[:relative] callback = proc do |modified, added, removed| @@ -61,7 +54,6 @@ def start listener = Listen.to( directory, - forward_to: address, relative: relative, &callback) diff --git a/lib/listen/listener.rb b/lib/listen/listener.rb index 2d15ce53..92a05be2 100644 --- a/lib/listen/listener.rb +++ b/lib/listen/listener.rb @@ -23,9 +23,6 @@ class Listener attr_reader :options, :directories attr_reader :registry, :supervisor - # TODO: deprecate - # NOTE: these are VERY confusing (broadcast + recipient modes) - attr_reader :host, :port # Initializes the directories listener. # @@ -50,13 +47,6 @@ def initialize(*args, &block) @silencer = Silencer.new _reconfigure_silencer({}) - @tcp_mode = nil - if [:recipient, :broadcaster].include? args[1] - target = args.shift - @tcp_mode = args.shift - _init_tcp_options(target) - end - @directories = args.flatten.map { |path| Pathname.new(path).realpath } @queue = Queue.new @block = block @@ -177,10 +167,6 @@ def queue(type, change, dir, path, options = {}) @last_queue_event_time = Time.now.to_f _wakeup_wait_thread unless state == :paused - return unless @tcp_mode == :broadcaster - - message = TCP::Message.new(type, change, dir, path, options) - registry[:broadcaster].async.broadcast(message.payload) end private @@ -308,23 +294,6 @@ def _process_changes attr_reader :wait_thread - def _init_tcp_options(target) - # Handle TCP options here - require 'listen/tcp' - fail ArgumentError, 'missing host/port for TCP' unless target - - if @tcp_mode == :recipient - @host = 'localhost' - @options[:force_tcp] = true - end - - if target.is_a? Fixnum - @port = target - else - @host, port = target.split(':') - @port = port.to_i - end - end def _reconfigure_silencer(extra_options) @options.merge!(extra_options) diff --git a/lib/listen/tcp.rb b/lib/listen/tcp.rb deleted file mode 100644 index c784c228..00000000 --- a/lib/listen/tcp.rb +++ /dev/null @@ -1,8 +0,0 @@ -begin - require 'celluloid/io' -rescue LoadError - Kernel.fail 'TCP forwarding requires Celluloid::IO to be present. ' \ - "Please install or add as a dependency: gem 'celluloid-io'" -end - -require 'listen/adapter/tcp' diff --git a/lib/listen/tcp/broadcaster.rb b/lib/listen/tcp/broadcaster.rb deleted file mode 100644 index 85d6fcb4..00000000 --- a/lib/listen/tcp/broadcaster.rb +++ /dev/null @@ -1,79 +0,0 @@ -require 'celluloid/io' - -module Listen - module TCP - class Broadcaster - include Celluloid::IO - - finalizer :finalize - - # Initializes a Celluloid::IO-powered TCP-broadcaster - # - # @param [String] host to broadcast on - # @param [String] port to broadcast on - # - # Note: Listens on all addresses when host is nil - # - def initialize(host, port) - @sockets = [] - _log :debug, format('Broadcaster: tcp server listening on: %s:%s', - host, port) - @server = TCPServer.new(host, port) - rescue - _log :error, format('Broadcaster.initialize: %s:%s', $ERROR_INFO, - $ERROR_POSITION * "\n") - raise - end - - # Asynchronously start accepting connections - def start - async.run - end - - # Cleans up sockets and server - def finalize - @sockets.map(&:close) if @sockets - @sockets = nil - - return unless @server - @server.close - @server = nil - end - - # Broadcasts given payload to all connected sockets - def broadcast(payload) - active_sockets = @sockets.select do |socket| - _unicast(socket, payload) - end - @sockets.replace(active_sockets) - end - - # Continuously accept and handle incoming connections - def run - while (socket = @server.accept) - @sockets << socket - end - rescue Celluloid::Task::TerminatedError - _log :debug, "TCP adapter was terminated: #{$ERROR_INFO}" - rescue - _log :error, format('Broadcaster.run: %s:%s', $ERROR_INFO, - $ERROR_POSITION * "\n") - raise - end - - private - - def _log(type, message) - Celluloid::Logger.send(type, message) - end - - def _unicast(socket, payload) - socket.write(payload) - true - rescue IOError, Errno::ECONNRESET, Errno::EPIPE - _log :debug, "Broadcaster failed: #{socket.inspect}" - false - end - end - end -end diff --git a/lib/listen/tcp/message.rb b/lib/listen/tcp/message.rb deleted file mode 100644 index eafd8b65..00000000 --- a/lib/listen/tcp/message.rb +++ /dev/null @@ -1,50 +0,0 @@ -require 'json' - -module Listen - module TCP - class Message - attr_reader :body, :object, :payload, :size - - HEADER_SIZE = 4 - HEADER_FORMAT = 'N' - PAYLOAD_FORMAT = "#{HEADER_FORMAT}a*" - - # Initializes a new message - # - # @param [Object] object to initialize message with - # - def initialize(*args) - self.object = args - end - - # Generates message size and payload for given object - def object=(obj) - @object = obj - @body = JSON.generate(@object) - @size = @body.bytesize - @payload = [@size, @body].pack(PAYLOAD_FORMAT) - end - - # Extracts message size and loads object from given payload - def payload=(payload) - @payload = payload - @size, @body = @payload.unpack(PAYLOAD_FORMAT) - @object = JSON.parse(@body) - end - - # Extracts a message from given buffer - def self.from_buffer(buffer) - if buffer.bytesize > HEADER_SIZE - size = buffer.unpack(HEADER_FORMAT).first - payload_size = HEADER_SIZE + size - if buffer.bytesize >= payload_size - payload = buffer.slice!(0...payload_size) - new.tap do |message| - message.payload = payload - end - end - end - end - end - end -end diff --git a/spec/acceptance/tcp_spec.rb b/spec/acceptance/tcp_spec.rb deleted file mode 100644 index d2551fc4..00000000 --- a/spec/acceptance/tcp_spec.rb +++ /dev/null @@ -1,136 +0,0 @@ -RSpec.describe Listen::Listener do - - let(:port) { 4000 } - let(:broadcast_options) { { forward_to: port } } - let(:paths) { Pathname.new(Dir.pwd) } - - around do |example| - fixtures { example.run } - Listen.stop - end - - modes = if !windows? || Celluloid::VERSION > '0.15.2' - [:recipient, :broadcaster] - else - [:broadcaster] - end - - modes.each do |mode| - context "when #{mode}" do - if mode == :broadcaster - subject { setup_listener(broadcast_options, :track_changes) } - before { subject.listener.start } - after { subject.listener.stop } - else - subject { setup_recipient(port, :track_changes) } - let(:broadcaster) { setup_listener(broadcast_options) } - - before do - broadcaster.listener.start - subject.lag = Float(ENV['LISTEN_TESTS_DEFAULT_TCP_LAG'] || 0.2) - subject.listener.start - end - after do - broadcaster.listener.stop - subject.listener.stop - end - end - - it { should process_addition_of('file.rb') } - - context 'when paused' do - before { subject.listener.pause } - - context 'with no queued changes' do - it { should_not process_addition_of('file.rb') } - - context 'when unpaused' do - before { subject.listener.unpause } - it { should process_addition_of('file.rb') } - end - end - - context 'with queued addition' do - before { change_fs(:added, 'file.rb') } - it { should_not process_modification_of('file.rb') } - - context 'when unpaused' do - before { subject.listener.unpause } - it { should process_queued_addition_of('file.rb') } - it { should process_modification_of('file.rb') } - end - end - - context 'with queued modification' do - before do - change_fs(:added, 'file.rb') - change_fs(:modified, 'file.rb') - end - - it { should_not process_queued_addition_of('file.rb') } - it { should_not process_queued_modification_of('file.rb') } - - context 'when unpaused' do - before { subject.listener.unpause } - it { should process_queued_addition_of('file.rb') } - - # NOTE: when adapter is 'local_fs?', the change optimizer - # (_squash_changes) reduces the "add+mod" into a single "add" - if mode == :broadcaster - # "optimizing" on local fs (broadcaster) will remove - # :modified from queue - it { should_not process_queued_modification_of('file.rb') } - else - # optimization skipped, because it's TCP, so we'll have both - # :modified and :added events for same file - it { should process_queued_modification_of('file.rb') } - end - - it { should process_modification_of('file.rb') } - end - end - end - - context 'when stopped' do - before { subject.listener.stop } - - context 'with no queued changes' do - it { should_not process_addition_of('file.rb') } - - context 'when started' do - before { subject.listener.start } - it { should process_addition_of('file.rb') } - end - end - - context 'with queued addition' do - before { change_fs(:added, 'file.rb') } - it { should_not process_modification_of('file.rb') } - - context 'when started' do - before { subject.listener.start } - it { should_not process_queued_addition_of('file.rb') } - it { should process_modification_of('file.rb') } - end - end - - context 'with queued modification' do - before do - change_fs(:added, 'file.rb') - change_fs(:modified, 'file.rb') - end - - it { should_not process_queued_addition_of('file.rb') } - it { should_not process_queued_modification_of('file.rb') } - - context 'when started' do - before { subject.listener.start } - it { should_not process_queued_addition_of('file.rb') } - it { should_not process_queued_modification_of('file.rb') } - it { should process_modification_of('file.rb') } - end - end - end - end - end -end diff --git a/spec/lib/listen/adapter/tcp_spec.rb b/spec/lib/listen/adapter/tcp_spec.rb deleted file mode 100644 index ea9b5cbe..00000000 --- a/spec/lib/listen/adapter/tcp_spec.rb +++ /dev/null @@ -1,127 +0,0 @@ -RSpec.describe Listen::Adapter::TCP do - - let(:host) { '10.0.0.2' } - let(:port) { 4000 } - - let(:options) { { host: host, port: port } } - - subject { described_class.new(options.merge(mq: listener)) } - let(:registry) { instance_double(Celluloid::Registry) } - - let(:listener) do - instance_double( - Listen::Listener, - registry: registry, - options: {}, - host: host, - port: port) - end - - let(:socket) do - instance_double(described_class::TCPSocket, close: true, recv: nil) - end - - before do - allow(described_class::TCPSocket).to receive(:new).and_return socket - end - - after do - subject.terminate - end - - describe '.usable?' do - it 'always returns true' do - expect(described_class).to be_usable - end - end - - describe '#start' do - it 'initializes and exposes a socket with listener host and port' do - expect(described_class::TCPSocket). - to receive(:new). - with listener.host, listener.port - - subject.start - expect(subject.socket).to be socket - end - - it 'initializes and exposes a string buffer' do - subject.start - expect(subject.buffer).to eq '' - end - - it 'invokes run loop' do - expect(subject.wrapped_object).to receive(:run) - subject.start - end - end - - describe '#finalize' do - it 'clears buffer' do - subject.start - subject.finalize - expect(subject.buffer).to be_nil - end - - it 'closes socket' do - subject.start - expect(subject.socket).to receive(:close) - subject.finalize - expect(subject.socket).to be_nil - end - end - - describe '#run' do - it 'handles data from socket' do - allow(socket).to receive(:recv).and_return 'foo', 'bar', nil - - expect_any_instance_of(described_class). - to receive(:handle_data).with('foo') - - expect_any_instance_of(described_class). - to receive(:handle_data).with('bar') - - subject.start - - # quick workaround because run is called asynchronously - sleep 0.5 - end - end - - describe '#handle_data' do - it 'buffers data' do - subject.start - subject.handle_data 'foo' - subject.handle_data 'bar' - expect(subject.buffer).to eq 'foobar' - end - - it 'handles messages accordingly' do - message = Listen::TCP::Message.new - - allow(Listen::TCP::Message).to receive(:from_buffer). - and_return message, nil - - expect(Listen::TCP::Message).to receive(:from_buffer).with 'foo' - expect(subject.wrapped_object).to receive(:handle_message).with message - - subject.start - subject.handle_data 'foo' - end - end - - describe '#handle_message' do - let(:dir) { Pathname.pwd } - it 'notifies listener of path changes' do - message = Listen::TCP::Message.new('file', 'modified', dir, 'foo', {}) - - expect(subject.wrapped_object). - to receive(:_queue_change).with :file, dir, 'foo', change: :modified - - subject.handle_message message - end - end - - specify { expect(described_class).to_not be_local_fs } - -end diff --git a/spec/lib/listen/adapter_spec.rb b/spec/lib/listen/adapter_spec.rb index d1b447d1..6b1fd92c 100644 --- a/spec/lib/listen/adapter_spec.rb +++ b/spec/lib/listen/adapter_spec.rb @@ -9,11 +9,6 @@ end describe '.select' do - it 'returns TCP adapter when requested' do - klass = Listen::Adapter.select(force_tcp: true) - expect(klass).to eq Listen::Adapter::TCP - end - it 'returns Polling adapter if forced' do klass = Listen::Adapter.select(force_polling: true) expect(klass).to eq Listen::Adapter::Polling diff --git a/spec/lib/listen/tcp/broadcaster_spec.rb b/spec/lib/listen/tcp/broadcaster_spec.rb deleted file mode 100644 index e638e909..00000000 --- a/spec/lib/listen/tcp/broadcaster_spec.rb +++ /dev/null @@ -1,122 +0,0 @@ -require 'listen/tcp/broadcaster' - -RSpec.describe Listen::TCP::Broadcaster do - - let(:host) { '10.0.0.2' } - let(:port) { 4000 } - - subject { described_class.new(host, port) } - - let(:server) do - instance_double(described_class::TCPServer, close: true, accept: nil) - end - - let(:socket) do - instance_double(described_class::TCPSocket, close: true, write: true) - end - - let(:socket2) do - instance_double(described_class::TCPSocket, close: true, write: true) - end - - let(:payload) { Listen::TCP::Message.new.payload } - - before do - expect(described_class::TCPServer).to receive(:new). - with(host, port).and_return server - allow(server).to receive(:accept).and_raise('stub called') - end - - after do - subject.terminate - end - - describe '#start' do - it 'invokes run loop asynchronously' do - expect_any_instance_of(described_class).to receive(:run) - subject.start - end - end - - describe '#finalize' do - before { allow(server).to receive(:accept).and_return nil } - - it 'closes server' do - expect(server).to receive(:close) - subject.finalize - end - end - - describe '#broadcast' do - context 'with active socket' do - before { allow(server).to receive(:accept).and_return socket, nil } - - it 'should broadcast payload' do - expect(socket).to receive(:write).with(payload) - subject.run - subject.broadcast payload - end - - it 'should keep socket' do - expect(socket).to receive(:write).twice.with(payload) - subject.run - 2.times { subject.broadcast payload } - end - - context 'with IOError' do - it 'should remove socket from list' do - allow(socket).to receive(:write).once.and_raise IOError - subject.run - 2.times { subject.broadcast payload } - end - end - - context 'when reset by peer' do - it 'should remove socket from list' do - allow(socket).to receive(:write).once.and_raise Errno::ECONNRESET - subject.run - 2.times { subject.broadcast payload } - end - end - - context 'when broken pipe' do - it 'should remove socket from list' do - allow(socket).to receive(:write).once.and_raise Errno::EPIPE - subject.run - 2.times { subject.broadcast payload } - end - end - - context 'with another active socket' do - before do - allow(server).to receive(:accept).and_return socket, socket2, nil - end - - it 'should broadcast payload to both' do - expect(socket).to receive(:write).with(payload) - expect(socket2).to receive(:write).with(payload) - subject.run - subject.broadcast payload - end - - context 'with a failure in first socket' do - before do - allow(socket).to receive(:write).once.and_raise Errno::EPIPE - end - - it 'should still broadcast to remaining socket' do - expect(socket2).to receive(:write).with(payload) - subject.run - subject.broadcast payload - end - - it 'should broadcast to only remaining socket' do - expect(socket2).to receive(:write).twice.with(payload) - subject.run - 2.times { subject.broadcast payload } - end - end - end - end - end -end diff --git a/spec/lib/listen/tcp/listener_spec.rb b/spec/lib/listen/tcp/listener_spec.rb deleted file mode 100644 index 98879f5d..00000000 --- a/spec/lib/listen/tcp/listener_spec.rb +++ /dev/null @@ -1,102 +0,0 @@ -require 'listen/tcp/message' -require 'listen/tcp/broadcaster' - -RSpec.describe Listen::Listener do - - let(:host) { '10.0.0.2' } - let(:port) { 4000 } - - subject { described_class.new("#{host}:#{port}", :recipient, options) } - let(:options) { {} } - let(:registry) { instance_double(Celluloid::Registry, :[]= => true) } - - let(:supervisor) do - instance_double(Celluloid::SupervisionGroup, add: true, pool: true) - end - - let(:record) { instance_double(Listen::Record, terminate: true, build: true) } - let(:silencer) { instance_double(Listen::Silencer, configure: nil) } - let(:adapter) { instance_double(Listen::Adapter::Base) } - let(:async) { instance_double(Listen::TCP::Broadcaster, broadcast: true) } - let(:broadcaster) { instance_double(Listen::TCP::Broadcaster, async: async) } - let(:change_pool) { instance_double(Listen::Change, terminate: true) } - let(:change_pool_async) { instance_double('ChangePoolAsync') } - before do - allow(Celluloid::Registry).to receive(:new) { registry } - allow(Celluloid::SupervisionGroup).to receive(:run!) { supervisor } - allow(registry).to receive(:[]).with(:adapter) { adapter } - allow(registry).to receive(:[]).with(:record) { record } - allow(registry).to receive(:[]).with(:change_pool) { change_pool } - allow(registry).to receive(:[]).with(:broadcaster) { broadcaster } - - allow(Listen::Silencer).to receive(:new) { silencer } - end - - describe '#initialize' do - it 'raises on omitted target' do - expect do - described_class.new(nil, :recipient) - end.to raise_error ArgumentError - end - end - - context 'when broadcaster' do - subject { described_class.new(port, :broadcaster) } - - it 'does not force TCP adapter through options' do - expect(subject.options).not_to include(force_tcp: true) - end - - describe '#start' do - before do - allow(subject).to receive(:_start_adapter) - allow(broadcaster).to receive(:start) - end - - it 'registers broadcaster' do - expect(supervisor).to receive(:add). - with(Listen::TCP::Broadcaster, as: :broadcaster, args: [nil, port]) - subject.start - end - - it 'starts broadcaster' do - expect(broadcaster).to receive(:start) - subject.start - end - end - - describe 'queue' do - before do - allow(broadcaster).to receive(:async).and_return async - end - - context 'when stopped' do - it 'honours stopped state and does nothing' do - allow(subject).to receive(:supervisor) do - instance_double(Celluloid::SupervisionGroup, terminate: true) - end - - subject.stop - subject.queue(:file, :modified, Pathname.pwd, 'foo') - expect(broadcaster).not_to receive(:async) - end - end - - let(:dir) { Pathname.pwd } - - it 'broadcasts changes asynchronously' do - message = Listen::TCP::Message.new(:file, :modified, dir, 'foo', {}) - expect(async).to receive(:broadcast).with message.payload - subject.queue(:file, :modified, Pathname.pwd, 'foo') - end - end - end - - context 'when recipient' do - subject { described_class.new(port, :recipient) } - - it 'forces TCP adapter through options' do - expect(subject.options).to include(force_tcp: true) - end - end -end diff --git a/spec/lib/listen/tcp/message_spec.rb b/spec/lib/listen/tcp/message_spec.rb deleted file mode 100644 index 9c5f555a..00000000 --- a/spec/lib/listen/tcp/message_spec.rb +++ /dev/null @@ -1,136 +0,0 @@ -# encoding: utf-8 - -RSpec.describe Listen::TCP::Message do - - let(:object) { [1, 2, { 'foo' => 'bar' }] } - let(:body) { '[1,2,{"foo":"bar"}]' } - let(:size) { 19 } - let(:payload) { "\x00\x00\x00\x13[1,2,{\"foo\":\"bar\"}]" } - - describe '#initialize' do - it 'initializes with an object' do - message = described_class.new(1, 2, 3) - expect(message.object).to eq [1, 2, 3] - end - end - - describe '#object=' do - subject do - described_class.new(object).tap do |message| - message.object = object - end - end - - describe '#object' do - subject { super().object } - it { is_expected.to be object } - end - - describe '#body' do - subject { super().body } - it { is_expected.to eq body } - end - - describe '#size' do - subject { super().size } - it { is_expected.to eq size } - end - - describe '#payload' do - subject { super().payload } - it { is_expected.to eq payload } - end - end - - describe '#payload=' do - subject do - described_class.new(object).tap do |message| - message.payload = payload - end - end - - describe '#object' do - subject { super().object } - it { is_expected.to eq object } - end - - describe '#body' do - subject { super().body } - it { is_expected.to eq body } - end - - describe '#size' do - subject { super().size } - it { is_expected.to eq size } - end - - describe '#payload' do - subject { super().payload } - it { is_expected.to be payload } - end - end - - describe '.from_buffer' do - - context 'when buffer is empty' do - it 'returns nil and leaves buffer intact' do - buffer = '' - message = described_class.from_buffer buffer - expect(message).to be_nil - expect(buffer).to eq '' - end - end - - context 'when buffer has data' do - - context 'with a partial packet' do - it 'returns nil and leaves remaining data intact' do - buffer = payload[0..4] - message = described_class.from_buffer buffer - expect(message).to be_nil - expect(buffer).to eq payload[0..4] - end - end - - context 'with a full packet' do - it 'extracts message from buffer and depletes buffer' do - buffer = payload.dup - message = described_class.from_buffer buffer - expect(message).to be_a described_class - expect(message.object).to eq object - expect(buffer).to eq '' - end - end - - context 'with a full and a partial packet' do - it 'extracts message from buffer and leaves remaining data intact' do - buffer = payload + payload[0..10] - message = described_class.from_buffer buffer - expect(message).to be_a described_class - expect(message.object).to eq object - expect(buffer).to eq payload[0..10] - end - end - - context 'with two full packets' do - it 'extracts both messages from buffer and depletes buffer' do - buffer = payload + payload - - message1 = described_class.from_buffer buffer - expect(message1).to be_a described_class - expect(message1.object).to eq object - - message2 = described_class.from_buffer buffer - expect(message2).to be_a described_class - expect(message2.object).to eq object - - expect(message1).not_to be message2 - expect(buffer).to eq '' - end - end - - end - - end - -end diff --git a/spec/lib/listen_spec.rb b/spec/lib/listen_spec.rb index 59ce90a7..56c319ac 100644 --- a/spec/lib/listen_spec.rb +++ b/spec/lib/listen_spec.rb @@ -10,41 +10,14 @@ expect(Listen::Listener).to receive(:new).with('/path') { listener } described_class.to('/path') end - - context 'when using :forward_to option' do - it 'initializes TCP-listener in broadcast-mode' do - expect(Listen::Listener).to receive(:new). - with(4000, :broadcaster, '/path', {}) { listener } - described_class.to('/path', forward_to: 4000) - end - end end describe '.stop' do - it 'stops all listeners & Celluloid' do + it 'stops all listeners' do allow(Listen::Listener).to receive(:new).with('/path') { listener } expect(listener).to receive(:stop) described_class.to('/path') Listen.stop - - # TODO: running? returns internal_pool on 0.15.2 - # (remove after Celluloid dependency is bumped) - buggy_method = if Celluloid.respond_to?(:internal_pool) - Celluloid.running? == Celluloid.internal_pool - else - false - end - - pool = buggy_method ? Celluloid.internal_pool : Celluloid - expect(pool).to_not be_running - end - end - - describe '.on' do - it 'initializes TCP-listener in recipient-mode' do - expect(Listen::Listener).to receive(:new). - with(4000, :recipient, '/path') { listener } - described_class.on(4000, '/path') end end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index a7624161..3cd425a0 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,5 +1,4 @@ require 'listen' -require 'listen/tcp' require 'listen/internals/thread_pool' From d0c29d4126a982c9e93df7909807a2fe79df7adb Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Fri, 12 Jun 2015 06:47:27 +0200 Subject: [PATCH 04/47] refactor record to contain one watched dir --- lib/listen/change.rb | 53 +++++--- lib/listen/directory.rb | 40 +++--- lib/listen/file.rb | 16 +-- lib/listen/listener.rb | 31 +++-- lib/listen/record.rb | 115 ++++++++---------- spec/lib/listen/change_spec.rb | 62 +++++----- spec/lib/listen/directory_spec.rb | 107 ++++++++-------- spec/lib/listen/file_spec.rb | 42 +++---- spec/lib/listen/listener_spec.rb | 12 +- spec/lib/listen/record_spec.rb | 195 +++++++++++++++--------------- 10 files changed, 329 insertions(+), 344 deletions(-) diff --git a/lib/listen/change.rb b/lib/listen/change.rb index ed49acca..76bbc3e1 100644 --- a/lib/listen/change.rb +++ b/lib/listen/change.rb @@ -3,19 +3,39 @@ module Listen class Change - include Celluloid + class Config + attr_reader :listener + def initialize(listener) + @listener = listener + end + + def silenced?(path, type) + listener.silencer.silenced?(Pathname(path), type) + end - attr_accessor :listener + def record_for(directory) + listener.record_for(directory) + end - def initialize(listener) - @listener = listener + def queue(*args) + listener.queue(*args) + end end - def change(type, watched_dir, rel_path, options = {}) + attr_reader :record + + def initialize(config, record) + @config = config + @record = record + end + + def change(type, rel_path, options) + watched_dir = Pathname.new(record.root) + change = options[:change] cookie = options[:cookie] - if !cookie && listener.silencer.silenced?(Pathname(rel_path), type) + if !cookie && config.silenced?(rel_path, type) _log :debug, "(silenced): #{rel_path.inspect}" return end @@ -26,24 +46,21 @@ def change(type, watched_dir, rel_path, options = {}) _log :debug, "#{log_details}: #{type}:#{path} (#{options.inspect})" if change - # TODO: move this to Listener to avoid Celluloid overhead + # TODO: move this to Listener to avoid overhead # from caller options = cookie ? { cookie: cookie } : {} - listener.queue(type, change, watched_dir, rel_path, options) + config.queue(type, change, watched_dir, rel_path, options) else - return unless (record = listener.sync(:record)) - if type == :dir - return unless (change_queue = listener.async(:change_pool)) - Directory.scan(change_queue, record, watched_dir, rel_path, options) + # NOTE: POSSIBLE RECURSION + # TODO: fix - use a queue instead + Directory.scan(self, rel_path, options) else - change = File.change(record, watched_dir, rel_path) + change = File.change(record, rel_path) return if !change || options[:silence] - listener.queue(:file, change, watched_dir, rel_path) + config.queue(:file, change, watched_dir, rel_path) end end - rescue Celluloid::Task::TerminatedError - _log :debug, "Change#change was terminated: #{$ERROR_INFO.inspect}" rescue RuntimeError _log :error, format('Change#change crashed %s:%s', $ERROR_INFO.inspect, $ERROR_POSITION * "\n") @@ -52,8 +69,10 @@ def change(type, watched_dir, rel_path, options = {}) private + attr_reader :config + def _log(type, message) - Celluloid::Logger.send(type, message) + Listen::Logger.send(type, message) end end end diff --git a/lib/listen/directory.rb b/lib/listen/directory.rb index e1185b4f..24e01d27 100644 --- a/lib/listen/directory.rb +++ b/lib/listen/directory.rb @@ -2,12 +2,12 @@ module Listen class Directory - def self.scan(queue, sync_record, dir, rel_path, options) - return unless (record = sync_record.async) + def self.scan(fs_change, rel_path, options) + record = fs_change.record + dir = Pathname.new(record.root) + previous = record.dir_entries(rel_path) - previous = sync_record.dir_entries(dir, rel_path) - - record.add_dir(dir, rel_path) + record.add_dir(rel_path) # TODO: use children(with_directory: false) path = dir + rel_path @@ -22,23 +22,23 @@ def self.scan(queue, sync_record, dir, rel_path, options) current.each do |full_path| type = full_path.directory? ? :dir : :file item_rel_path = full_path.relative_path_from(dir).to_s - _change(queue, type, dir, item_rel_path, options) + _change(fs_change, type, item_rel_path, options) end # TODO: this is not tested properly previous = previous.reject { |entry, _| current.include? path + entry } - _async_changes(dir, rel_path, queue, previous, options) + _async_changes(fs_change, Pathname.new(rel_path), previous, options) rescue Errno::ENOENT, Errno::EHOSTDOWN - record.unset_path(dir, rel_path) - _async_changes(dir, rel_path, queue, previous, options) + record.unset_path(rel_path) + _async_changes(fs_change, Pathname.new(rel_path), previous, options) rescue Errno::ENOTDIR # TODO: path not tested - record.unset_path(dir, rel_path) - _async_changes(dir, path, queue, previous, options) - _change(queue, :file, dir, rel_path, options) + record.unset_path(rel_path) + _async_changes(fs_change, path, previous, options) + _change(fs_change, :file, rel_path, options) rescue _log(:warn) do format('scan DIED: %s:%s', $ERROR_INFO, $ERROR_POSITION * "\n") @@ -46,26 +46,24 @@ def self.scan(queue, sync_record, dir, rel_path, options) raise end - def self._async_changes(dir, path, queue, previous, options) + def self._async_changes(fs_change, path, previous, options) + fail "Not a Pathname: #{path.inspect}" unless path.respond_to?(:children) previous.each do |entry, data| # TODO: this is a hack with insufficient testing type = data.key?(:mtime) ? :file : :dir - _change(queue, type, dir, (Pathname(path) + entry).to_s, options) + rel_path_s = (path + entry).to_s + _change(fs_change, type, rel_path_s, options) end end - def self._change(queue, type, dir, path, options) - return queue.change(type, dir, path, options) if type == :dir + def self._change(fs_change, type, path, options) + return fs_change.change(type, path, options) if type == :dir # Minor param cleanup for tests # TODO: use a dedicated Event class opts = options.dup opts.delete(:recursive) - if opts.empty? - queue.change(type, dir, path) - else - queue.change(type, dir, path, opts) - end + fs_change.change(type, path, opts) end def self._log(type, &block) diff --git a/lib/listen/file.rb b/lib/listen/file.rb index 94f7ddb0..b2bb2b84 100644 --- a/lib/listen/file.rb +++ b/lib/listen/file.rb @@ -1,25 +1,25 @@ module Listen class File - def self.change(record, dir, rel_path) - path = dir + rel_path + def self.change(record, rel_path) + path = Pathname.new(record.root) + rel_path lstat = path.lstat data = { mtime: lstat.mtime.to_f, mode: lstat.mode } - record_data = record.file_data(dir, rel_path) + record_data = record.file_data(rel_path) if record_data.empty? - record.async.update_file(dir, rel_path, data) + record.update_file(rel_path, data) return :added end if data[:mode] != record_data[:mode] - record.async.update_file(dir, rel_path, data) + record.update_file(rel_path, data) return :modified end if data[:mtime] != record_data[:mtime] - record.async.update_file(dir, rel_path, data) + record.update_file(rel_path, data) return :modified end @@ -56,10 +56,10 @@ def self.change(record, dir, rel_path) return if data[:mtime].to_i + 2 <= Time.now.to_f md5 = Digest::MD5.file(path).digest - record.async.update_file(dir, rel_path, data.merge(md5: md5)) + record.update_file(rel_path, data.merge(md5: md5)) :modified if record_data[:md5] && md5 != record_data[:md5] rescue SystemCallError - record.async.unset_path(dir, rel_path) + record.unset_path(rel_path) :removed rescue Celluloid::Logger.debug "lstat failed for: #{rel_path} (#{$ERROR_INFO})" diff --git a/lib/listen/listener.rb b/lib/listen/listener.rb index 92a05be2..18b13b84 100644 --- a/lib/listen/listener.rb +++ b/lib/listen/listener.rb @@ -76,9 +76,16 @@ def initialize(*args, &block) _start_wait_thread _init_actors - # Note: make sure building is finished before starting adapter (for - # consistent results both in specs and normal usage) - sync(:record).build + begin + start = Time.now.to_f + # Note: make sure building is finished before starting adapter (for + # consistent results both in specs and normal usage) + fs_changes.values.map(&:record).map(&:build) + Listen::Logger.info "Record.build(): #{Time.now.to_f - start} seconds" + rescue + Listen::Logger.warn "build crashed: #{$ERROR_INFO.inspect}" + raise + end _start_adapter end @@ -166,7 +173,10 @@ def queue(type, change, dir, path, options = {}) @last_queue_event_time = Time.now.to_f _wakeup_wait_thread unless state == :paused + end + def record_for(dir) + fs_changes[dir.to_s].record end private @@ -203,8 +213,6 @@ def _init_actors adapter_options = { mq: self, directories: directories } @supervisor = Celluloid::SupervisionGroup.run!(registry) - supervisor.add(Record, as: :record, args: self) - supervisor.pool(Change, as: :change_pool, args: self) # TODO: broadcaster should be a separate plugin if @tcp_mode == :broadcaster @@ -292,6 +300,11 @@ def _process_changes _debug "Callback took #{Time.now.to_f - block_start} seconds" end + attr_reader :adapter + attr_reader :queue_optimizer + attr_reader :event_queue + attr_reader :fs_changes + attr_reader :wait_thread @@ -325,13 +338,7 @@ def _stop_wait_thread def _queue_raw_change(type, dir, rel_path, options) _debug { "raw queue: #{[type, dir, rel_path, options].inspect}" } - - unless (worker = async(:change_pool)) - _warn 'Failed to allocate worker from change pool' - return - end - - worker.change(type, dir, rel_path, options) + fs_changes[dir.to_s].change(type, rel_path, options) rescue RuntimeError _error_exception "_queue_raw_change exception %s:\n%s:\n" raise diff --git a/lib/listen/record.rb b/lib/listen/record.rb index 3255fc45..41bc9990 100644 --- a/lib/listen/record.rb +++ b/lib/listen/record.rb @@ -3,55 +3,53 @@ module Listen class Record - include Celluloid # TODO: one Record object per watched directory? - # TODO: deprecate - attr_accessor :paths, :listener - def initialize(listener) - @listener = listener - @paths = _auto_hash + attr_reader :root + def initialize(directory) + @tree = _auto_hash + @root = directory.to_s end - def add_dir(dir, rel_path) + def add_dir(rel_path) return if [nil, '', '.'].include? rel_path - @paths[dir.to_s][rel_path] ||= {} + @tree[rel_path] ||= {} end - def update_file(dir, rel_path, data) + def update_file(rel_path, data) dirname, basename = Pathname(rel_path).split.map(&:to_s) - _fast_update_file(dir, dirname, basename, data) + _fast_update_file(dirname, basename, data) end - def unset_path(dir, rel_path) + def unset_path(rel_path) dirname, basename = Pathname(rel_path).split.map(&:to_s) - _fast_unset_path(dir, dirname, basename) + _fast_unset_path(dirname, basename) end - def file_data(dir, rel_path) - root = @paths[dir.to_s] + def file_data(rel_path) dirname, basename = Pathname(rel_path).split.map(&:to_s) if [nil, '', '.'].include? dirname - root[basename] ||= {} - root[basename].dup + tree[basename] ||= {} + tree[basename].dup else - root[dirname] ||= {} - root[dirname][basename] ||= {} - root[dirname][basename].dup + tree[dirname] ||= {} + tree[dirname][basename] ||= {} + tree[dirname][basename].dup end end - def dir_entries(dir, rel_path) - tree = if [nil, '', '.'].include? rel_path.to_s - @paths[dir.to_s] - else - @paths[dir.to_s][rel_path.to_s] ||= _auto_hash - @paths[dir.to_s][rel_path.to_s] - end + def dir_entries(rel_path) + subtree = + if [nil, '', '.'].include? rel_path.to_s + tree + else + tree[rel_path.to_s] ||= _auto_hash + tree[rel_path.to_s] + end result = {} - tree.each do |key, values| + subtree.each do |key, values| # only get data for file entries result[key] = values.key?(:mtime) ? values : {} end @@ -59,18 +57,14 @@ def dir_entries(dir, rel_path) end def build - start = Time.now.to_f - @paths = _auto_hash - - # TODO: refactor this out (1 Record = 1 watched dir) - listener.directories.each do |directory| - _fast_build(directory.to_s) - end - - Celluloid::Logger.info "Record.build(): #{Time.now.to_f - start} seconds" - rescue - Celluloid::Logger.warn "build crashed: #{$ERROR_INFO.inspect}" - raise + @tree = _auto_hash + # TODO: test with a file name given + # TODO: test other permissions + # TODO: test with mixed encoding + symlink_detector = SymlinkDetector.new + remaining = Queue.new + remaining << Entry.new(root, nil, nil) + _fast_build_dir(remaining, symlink_detector) until remaining.empty? end private @@ -79,56 +73,47 @@ def _auto_hash Hash.new { |h, k| h[k] = Hash.new } end - def _fast_update_file(dir, dirname, basename, data) - root = @paths[dir.to_s] + def tree + @tree + end + + def _fast_update_file(dirname, basename, data) if [nil, '', '.'].include? dirname - root[basename] = (root[basename] || {}).merge(data) + tree[basename] = (tree[basename] || {}).merge(data) else - root[dirname] ||= {} - root[dirname][basename] = (root[dirname][basename] || {}).merge(data) + tree[dirname] ||= {} + tree[dirname][basename] = (tree[dirname][basename] || {}).merge(data) end end - def _fast_unset_path(dir, dirname, basename) - root = @paths[dir.to_s] + def _fast_unset_path(dirname, basename) # this may need to be reworked to properly remove # entries from a tree, without adding non-existing dirs to the record if [nil, '', '.'].include? dirname - return unless root.key?(basename) - root.delete(basename) + return unless tree.key?(basename) + tree.delete(basename) else - return unless root.key?(dirname) - root[dirname].delete(basename) + return unless tree.key?(dirname) + tree[dirname].delete(basename) end end - # TODO: test with a file name given - # TODO: test other permissions - # TODO: test with mixed encoding - def _fast_build(root) - symlink_detector = SymlinkDetector.new - @paths[root] = _auto_hash - remaining = Queue.new - remaining << Entry.new(root, nil, nil) - _fast_build_dir(remaining, symlink_detector) until remaining.empty? - end - def _fast_build_dir(remaining, symlink_detector) entry = remaining.pop children = entry.children # NOTE: children() implicitly tests if dir symlink_detector.verify_unwatched!(entry) children.each { |child| remaining << child } - add_dir(entry.root, entry.record_dir_key) + add_dir(entry.record_dir_key) rescue Errno::ENOTDIR _fast_try_file(entry) rescue SystemCallError, SymlinkDetector::Error - _fast_unset_path(entry.root, entry.relative, entry.name) + _fast_unset_path(entry.relative, entry.name) end def _fast_try_file(entry) - _fast_update_file(entry.root, entry.relative, entry.name, entry.meta) + _fast_update_file(entry.relative, entry.name, entry.meta) rescue SystemCallError - _fast_unset_path(entry.root, entry.relative, entry.name) + _fast_unset_path(entry.relative, entry.name) end end end diff --git a/spec/lib/listen/change_spec.rb b/spec/lib/listen/change_spec.rb index 09d9c1a3..5b7fc00e 100644 --- a/spec/lib/listen/change_spec.rb +++ b/spec/lib/listen/change_spec.rb @@ -1,60 +1,60 @@ RSpec.describe Listen::Change do - let(:subject) { Listen::Change.new(listener) } - let(:listener) { instance_double(Listen::Listener, options: {}) } - let(:record) { instance_double(Listen::Record) } + let(:config) { instance_double(Listen::Change::Config) } + let(:dir) { instance_double(Pathname) } + let(:record) { instance_double(Listen::Record, root: '/dir') } + subject { Listen::Change.new(config, record) } + let(:full_file_path) { instance_double(Pathname, to_s: '/dir/file.rb') } let(:full_dir_path) { instance_double(Pathname, to_s: '/dir') } - let(:dir) { instance_double(Pathname) } before do - allow(listener).to receive(:sync).with(:record) { record } - allow(listener).to receive(:async).with(:change_pool) { subject } allow(dir).to receive(:+).with('file.rb') { full_file_path } allow(dir).to receive(:+).with('dir1') { full_dir_path } end describe '#change' do - let(:silencer) { instance_double(Listen::Silencer, silenced?: false) } - before { allow(listener).to receive(:silencer) { silencer } } + before do + allow(config).to receive(:silenced?).and_return(false) + end context 'with build options' do it 'calls still_building! on record' do - allow(listener).to receive(:queue) - allow(record).to receive(:async) { async_record } + allow(config).to receive(:queue) + allow(config).to receive(:record_for).with(dir).and_return(record) allow(Listen::File).to receive(:change) - subject.change(:file, dir, 'file.rb', build: true) + subject.change(:file, 'file.rb', build: true) end end context 'file' do context 'with known change' do it 'notifies change directly to listener' do - expect(listener).to receive(:queue). - with(:file, :modified, dir, 'file.rb', {}) + expect(config).to receive(:queue). + with(:file, :modified, Pathname.new('/dir'), 'file.rb', {}) - subject.change(:file, dir, 'file.rb', change: :modified) + subject.change(:file, 'file.rb', change: :modified) end it "doesn't notify to listener if path is silenced" do - expect(silencer).to receive(:silenced?).and_return(true) - expect(listener).to_not receive(:queue) - subject.change(:file, dir, 'file.rb', change: :modified) + expect(config).to receive(:silenced?).and_return(true) + expect(config).to_not receive(:queue) + subject.change(:file, 'file.rb', change: :modified) end end context 'with unknown change' do it 'calls Listen::File#change' do - expect(Listen::File).to receive(:change).with(record, dir, 'file.rb') - subject.change(:file, dir, 'file.rb') + expect(Listen::File).to receive(:change).with(record, 'file.rb') + subject.change(:file, 'file.rb', {}) end it "doesn't call Listen::File#change if path is silenced" do - expect(silencer).to receive(:silenced?). - with(Pathname('file.rb'), :file).and_return(true) + expect(config).to receive(:silenced?). + with('file.rb', :file).and_return(true) expect(Listen::File).to_not receive(:change) - subject.change(:file, dir, 'file.rb') + subject.change(:file, 'file.rb', {}) end context 'that returns a change' do @@ -62,16 +62,16 @@ context 'listener listen' do it 'notifies change to listener' do - expect(listener).to receive(:queue). - with(:file, :modified, dir, 'file.rb') + expect(config).to receive(:queue). + with(:file, :modified, Pathname.new('/dir'), 'file.rb') - subject.change(:file, dir, 'file.rb') + subject.change(:file, 'file.rb', {}) end context 'silence option' do it 'notifies change to listener' do - expect(listener).to_not receive(:queue) - subject.change(:file, dir, 'file.rb', silence: true) + expect(config).to_not receive(:queue) + subject.change(:file, 'file.rb', silence: true) end end end @@ -81,8 +81,8 @@ before { allow(Listen::File).to receive(:change) { nil } } it "doesn't notifies no change" do - expect(listener).to_not receive(:queue) - subject.change(:file, dir, 'file.rb') + expect(config).to_not receive(:queue) + subject.change(:file, 'file.rb', {}) end end end @@ -93,9 +93,9 @@ it 'calls Listen::Directory#new' do expect(Listen::Directory).to receive(:scan). - with(subject, record, dir, 'dir1', dir_options) + with(subject, 'dir1', dir_options) - subject.change(:dir, dir, 'dir1', dir_options) + subject.change(:dir, 'dir1', dir_options) end end end diff --git a/spec/lib/listen/directory_spec.rb b/spec/lib/listen/directory_spec.rb index f77e98ce..28cc5226 100644 --- a/spec/lib/listen/directory_spec.rb +++ b/spec/lib/listen/directory_spec.rb @@ -2,20 +2,16 @@ RSpec.describe Directory do let(:dir) { double(:dir) } - let(:file) { double(:file, directory?: false) } - let(:file2) { double(:file2, directory?: false) } - let(:subdir) { double(:subdir, directory?: true) } - - let(:queue) { instance_double(Change, change: nil) } - - let(:async_record) do - instance_double(Record, add_dir: true, unset_path: true) - end + let(:file) { double(:file, directory?: false, to_s: 'file.rb') } + let(:file2) { double(:file2, directory?: false, to_s: 'file2.rb') } + let(:subdir) { double(:subdir, directory?: true, to_s: 'subdir') } let(:record) do - instance_double(Record, async: async_record, dir_entries: record_entries) + instance_double(Record, root: 'some_dir', dir_entries: record_entries, add_dir: true, unset_path: true) end + let(:fs_change) { instance_double(Change, change: nil, record: record) } + before do allow(dir).to receive(:+).with('.') { dir } allow(dir).to receive(:+).with('file.rb') { file } @@ -24,6 +20,9 @@ allow(file).to receive(:relative_path_from).with(dir) { 'file.rb' } allow(file2).to receive(:relative_path_from).with(dir) { 'file2.rb' } allow(subdir).to receive(:relative_path_from).with(dir) { 'subdir' } + + allow(Pathname).to receive(:new).with('some_dir').and_return(dir) + allow(Pathname).to receive(:new).with('.').and_return(dir) end context '#scan with recursive off' do @@ -38,17 +37,15 @@ before { allow(dir).to receive(:children) { [] } } it 'sets record dir path' do - expect(async_record).to receive(:add_dir).with(dir, '.') - described_class.scan(queue, record, dir, '.', options) + expect(record).to receive(:add_dir).with('.') + described_class.scan(fs_change, '.', options) end - it "queues changes for file path and dir that doesn't exist" do - expect(queue).to receive(:change).with(:file, dir, 'file.rb') - - expect(queue).to receive(:change). - with(:dir, dir, 'subdir', recursive: false) + it "fs_changes changes for file path and dir that doesn't exist" do + expect(fs_change).to receive(:change).with(:file, 'file.rb', {}) + expect(fs_change).to receive(:change).with(:dir, 'subdir', recursive: false) - described_class.scan(queue, record, dir, '.', options) + described_class.scan(fs_change, '.', options) end end @@ -56,13 +53,11 @@ before { allow(dir).to receive(:children) { [file2] } } it 'notices file & file2 and no longer existing dir' do - expect(queue).to receive(:change).with(:file, dir, 'file.rb') - expect(queue).to receive(:change).with(:file, dir, 'file2.rb') + expect(fs_change).to receive(:change).with(:file, 'file.rb', {}) + expect(fs_change).to receive(:change).with(:file, 'file2.rb', {}) + expect(fs_change).to receive(:change).with(:dir, 'subdir', recursive: false) - expect(queue).to receive(:change). - with(:dir, dir, 'subdir', recursive: false) - - described_class.scan(queue, record, dir, '.', options) + described_class.scan(fs_change, '.', options) end end end @@ -74,13 +69,13 @@ before { allow(dir).to receive(:children) { fail Errno::ENOENT } } it 'reports no changes' do - expect(queue).to_not receive(:change) - described_class.scan(queue, record, dir, '.', options) + expect(fs_change).to_not receive(:change) + described_class.scan(fs_change, '.', options) end it 'unsets record dir path' do - expect(async_record).to receive(:unset_path).with(dir, '.') - described_class.scan(queue, record, dir, '.', options) + expect(record).to receive(:unset_path).with('.') + described_class.scan(fs_change, '.', options) end end @@ -88,27 +83,25 @@ before { allow(dir).to receive(:children) { fail Errno::EHOSTDOWN } } it 'reports no changes' do - expect(queue).to_not receive(:change) - described_class.scan(queue, record, dir, '.', options) + expect(fs_change).to_not receive(:change) + described_class.scan(fs_change, '.', options) end it 'unsets record dir path' do - expect(async_record).to receive(:unset_path).with(dir, '.') - described_class.scan(queue, record, dir, '.', options) + expect(record).to receive(:unset_path).with('.') + described_class.scan(fs_change, '.', options) end end context 'with file.rb in dir' do before { allow(dir).to receive(:children) { [file] } } - it 'queues changes for file & file2 paths' do - expect(queue).to receive(:change).with(:file, dir, 'file.rb') - expect(queue).to_not receive(:change).with(:file, dir, 'file2.rb') - - expect(queue).to_not receive(:change). - with(:dir, dir, 'subdir', recursive: false) + it 'fs_changes changes for file & file2 paths' do + expect(fs_change).to receive(:change).with(:file, 'file.rb', {}) + expect(fs_change).to_not receive(:change).with(:file,'file2.rb', {}) + expect(fs_change).to_not receive(:change).with(:dir, 'subdir', recursive: false) - described_class.scan(queue, record, dir, '.', options) + described_class.scan(fs_change, '.', options) end end end @@ -127,13 +120,13 @@ allow(dir).to receive(:children) { [] } end - it 'queues changes for file & subdir path' do - expect(queue).to receive(:change).with(:file, dir, 'file.rb') + it 'fs_changes changes for file & subdir path' do + expect(fs_change).to receive(:change).with(:file, 'file.rb', {}) - expect(queue).to receive(:change). - with(:dir, dir, 'subdir', recursive: true) + expect(fs_change).to receive(:change). + with(:dir, 'subdir', recursive: true) - described_class.scan(queue, record, dir, '.', options) + described_class.scan(fs_change, '.', options) end end @@ -145,16 +138,16 @@ allow(subdir2).to receive(:relative_path_from).with(dir) { 'subdir2' } end - it 'queues changes for file, file2 & subdir paths' do - expect(queue).to receive(:change).with(:file, dir, 'file.rb') + it 'fs_changes changes for file, file2 & subdir paths' do + expect(fs_change).to receive(:change).with(:file, 'file.rb', {}) - expect(queue).to receive(:change). - with(:dir, dir, 'subdir', recursive: true) + expect(fs_change).to receive(:change). + with(:dir, 'subdir', recursive: true) - expect(queue).to receive(:change). - with(:dir, dir, 'subdir2', recursive: true) + expect(fs_change).to receive(:change). + with(:dir, 'subdir2', recursive: true) - described_class.scan(queue, record, dir, '.', options) + described_class.scan(fs_change, '.', options) end end end @@ -168,8 +161,8 @@ end it 'reports no changes' do - expect(queue).to_not receive(:change) - described_class.scan(queue, record, dir, '.', options) + expect(fs_change).to_not receive(:change) + described_class.scan(fs_change, '.', options) end end @@ -180,11 +173,11 @@ allow(subdir).to receive(:children) { [] } end - it 'queues changes for subdir' do - expect(queue).to receive(:change). - with(:dir, dir, 'subdir', recursive: true) + it 'fs_changes changes for subdir' do + expect(fs_change).to receive(:change). + with(:dir, 'subdir', recursive: true) - described_class.scan(queue, record, dir, '.', options) + described_class.scan(fs_change, '.', options) end end end diff --git a/spec/lib/listen/file_spec.rb b/spec/lib/listen/file_spec.rb index 78f474f8..aa0a9e6d 100644 --- a/spec/lib/listen/file_spec.rb +++ b/spec/lib/listen/file_spec.rb @@ -1,23 +1,17 @@ RSpec.describe Listen::File do - let(:async_record) do + let(:record) do instance_double( Listen::Record, + root: '/foo/bar', + file_data: record_data, add_dir: true, update_file: true, - unset_path: true - ) - end - - let(:record) do - instance_double( - Listen::Record, - async: async_record, - file_data: record_data + unset_path: true, ) end let(:path) { Pathname.pwd } - let(:subject) { described_class.change(record, path, 'file.rb') } + let(:subject) { described_class.change(record, 'file.rb') } around { |example| fixtures { example.run } } @@ -40,10 +34,10 @@ context 'with non-existing file' do before { allow(::File).to receive(:lstat) { fail Errno::ENOENT } } - it { should be :removed } + it { is_expected.to eq(:removed) } it 'sets path in record' do - expect(async_record).to receive(:unset_path).with(path, 'file.rb') + expect(record).to receive(:unset_path).with('file.rb') subject end end @@ -76,8 +70,7 @@ it { should be :modified } it 'sets path in record with expected data' do - expect(async_record).to receive(:update_file). - with(path, 'file.rb', expected_data) + expect(record).to receive(:update_file).with('file.rb', expected_data) subject end end @@ -92,8 +85,8 @@ it { should be :modified } it 'sets path in record with expected data' do - expect(async_record).to receive(:update_file). - with(path, 'file.rb', expected_data) + expect(record).to receive(:update_file). + with('file.rb', expected_data) subject end end @@ -104,8 +97,8 @@ it { should be :modified } it 'sets path in record with expected data' do - expect(async_record).to receive(:update_file). - with(path, 'file.rb', expected_data) + expect(record).to receive(:update_file). + with('file.rb', expected_data) subject end end @@ -155,7 +148,7 @@ it { should be :removed } it 'should not unset record' do - expect(async_record).to_not receive(:unset_path) + expect(record).to_not receive(:unset_path) end end @@ -177,8 +170,9 @@ it { should be :modified } it 'sets path in record with expected data' do - expect(async_record).to receive(:update_file). - with(path, 'file.rb', expected_data. merge(md5: md5)) + expected = expected_data. merge(md5: md5) + expect(record).to receive(:update_file). + with('file.rb', expected) subject end end @@ -211,8 +205,8 @@ end it 'sets path in record with expected data' do - expect(async_record).to receive(:update_file). - with(path, 'file.rb', expected_data) + expect(record).to receive(:update_file). + with('file.rb', expected_data) subject end end diff --git a/spec/lib/listen/listener_spec.rb b/spec/lib/listen/listener_spec.rb index 248f3bbe..4a79fa69 100644 --- a/spec/lib/listen/listener_spec.rb +++ b/spec/lib/listen/listener_spec.rb @@ -114,13 +114,6 @@ allow(silencer).to receive(:silenced?) { false } end - it 'supervises change_pool' do - expect(supervisor).to receive(:pool). - with(Change, as: :change_pool, args: subject) - - subject.start - end - it 'supervises adapter' do allow(Adapter).to receive(:select) { Adapter::Polling } options = [mq: subject, directories: []] @@ -579,13 +572,10 @@ context 'when listener is stopped' do before do - allow(registry).to receive(:[]).with(:change_pool) { nil } subject.stop end - let(:dir) { instance_double(Pathname) } - - it 'queuing does not crash when no worker is available' do + it 'queuing does not crash when changes come in' do expect do subject.send(:_queue_raw_change, :dir, dir, 'path', recursive: true) end.to_not raise_error diff --git a/spec/lib/listen/record_spec.rb b/spec/lib/listen/record_spec.rb index 8e2ea9e0..08e0939a 100644 --- a/spec/lib/listen/record_spec.rb +++ b/spec/lib/listen/record_spec.rb @@ -1,9 +1,6 @@ RSpec.describe Listen::Record do - let(:registry) { instance_double(Celluloid::Registry) } - - let(:listener) do - instance_double(Listen::Listener, registry: registry, options: {}) - end + let(:dir) { instance_double(Pathname, to_s: '/dir') } + let(:record) { Listen::Record.new(dir) } def dir_entries_for(hash) hash.each do |dir, entries| @@ -45,34 +42,35 @@ def symlink(hash_or_dir) end end - let(:record) { Listen::Record.new(listener) } - let(:dir) { instance_double(Pathname, to_s: '/dir') } + def record_tree(record) + record.instance_variable_get(:@tree) + end describe '#update_file' do context 'with path in watched dir' do it 'sets path by spliting dirname and basename' do - record.update_file(dir, 'file.rb', mtime: 1.1) - expect(record.paths['/dir']).to eq('file.rb' => { mtime: 1.1 }) + record.update_file('file.rb', mtime: 1.1) + expect(record_tree(record)).to eq('file.rb' => { mtime: 1.1 }) end it 'sets path and keeps old data not overwritten' do - record.update_file(dir, 'file.rb', foo: 1, bar: 2) - record.update_file(dir, 'file.rb', foo: 3) - watched_dir = record.paths['/dir'] + record.update_file('file.rb', foo: 1, bar: 2) + record.update_file('file.rb', foo: 3) + watched_dir = record_tree(record) expect(watched_dir).to eq('file.rb' => { foo: 3, bar: 2 }) end end context 'with subdir path' do it 'sets path by spliting dirname and basename' do - record.update_file(dir, 'path/file.rb', mtime: 1.1) - expect(record.paths['/dir']['path']).to eq('file.rb' => { mtime: 1.1 }) + record.update_file('path/file.rb', mtime: 1.1) + expect(record_tree(record)['path']).to eq('file.rb' => { mtime: 1.1 }) end it 'sets path and keeps old data not overwritten' do - record.update_file(dir, 'path/file.rb', foo: 1, bar: 2) - record.update_file(dir, 'path/file.rb', foo: 3) - file_data = record.paths['/dir']['path']['file.rb'] + record.update_file('path/file.rb', foo: 1, bar: 2) + record.update_file('path/file.rb', foo: 3) + file_data = record_tree(record)['path']['file.rb'] expect(file_data).to eq(foo: 3, bar: 2) end end @@ -80,33 +78,33 @@ def symlink(hash_or_dir) describe '#add_dir' do it 'sets itself when .' do - record.add_dir(dir, '.') - expect(record.paths['/dir']).to eq({}) + record.add_dir('.') + expect(record_tree(record)).to eq({}) end it 'sets itself when nil' do - record.add_dir(dir, nil) - expect(record.paths['/dir']).to eq({}) + record.add_dir(nil) + expect(record_tree(record)).to eq({}) end it 'sets itself when empty' do - record.add_dir(dir, '') - expect(record.paths['/dir']).to eq({}) + record.add_dir('') + expect(record_tree(record)).to eq({}) end it 'correctly sets new directory data' do - record.add_dir(dir, 'path/subdir') - expect(record.paths['/dir']).to eq('path/subdir' => {}) + record.add_dir('path/subdir') + expect(record_tree(record)).to eq('path/subdir' => {}) end it 'sets path and keeps old data not overwritten' do - record.add_dir(dir, 'path/subdir') - record.update_file(dir, 'path/subdir/file.rb', mtime: 1.1) - record.add_dir(dir, 'path/subdir') - record.update_file(dir, 'path/subdir/file2.rb', mtime: 1.2) - record.add_dir(dir, 'path/subdir') + record.add_dir('path/subdir') + record.update_file('path/subdir/file.rb', mtime: 1.1) + record.add_dir('path/subdir') + record.update_file('path/subdir/file2.rb', mtime: 1.2) + record.add_dir('path/subdir') - watched = record.paths['/dir'] + watched = record_tree(record) expect(watched.keys).to eq ['path/subdir'] expect(watched['path/subdir'].keys).to eq %w(file.rb file2.rb) @@ -119,36 +117,36 @@ def symlink(hash_or_dir) describe '#unset_path' do context 'within watched dir' do context 'when path is present' do - before { record.update_file(dir, 'file.rb', mtime: 1.1) } + before { record.update_file('file.rb', mtime: 1.1) } it 'unsets path' do - record.unset_path(dir, 'file.rb') - expect(record.paths).to eq('/dir' => {}) + record.unset_path('file.rb') + expect(record_tree(record)).to eq({}) end end context 'when path not present' do it 'unsets path' do - record.unset_path(dir, 'file.rb') - expect(record.paths).to eq('/dir' => {}) + record.unset_path('file.rb') + expect(record_tree(record)).to eq({}) end end end context 'within subdir' do context 'when path is present' do - before { record.update_file(dir, 'path/file.rb', mtime: 1.1) } + before { record.update_file('path/file.rb', mtime: 1.1) } it 'unsets path' do - record.unset_path(dir, 'path/file.rb') - expect(record.paths).to eq('/dir' => { 'path' => {} }) + record.unset_path('path/file.rb') + expect(record_tree(record)).to eq({ 'path' => {} }) end end context 'when path not present' do it 'unsets path' do - record.unset_path(dir, 'path/file.rb') - expect(record.paths).to eq('/dir' => {}) + record.unset_path('path/file.rb') + expect(record_tree(record)).to eq({}) end end end @@ -157,33 +155,33 @@ def symlink(hash_or_dir) describe '#file_data' do context 'with path in watched dir' do context 'when path is present' do - before { record.update_file(dir, 'file.rb', mtime: 1.1) } + before { record.update_file('file.rb', mtime: 1.1) } it 'returns file data' do - expect(record.file_data(dir, 'file.rb')).to eq(mtime: 1.1) + expect(record.file_data('file.rb')).to eq(mtime: 1.1) end end context 'path not present' do it 'return empty hash' do - expect(record.file_data(dir, 'file.rb')).to be_empty + expect(record.file_data('file.rb')).to be_empty end end end context 'with path in subdir' do context 'when path is present' do - before { record.update_file(dir, 'path/file.rb', mtime: 1.1) } + before { record.update_file('path/file.rb', mtime: 1.1) } it 'returns file data' do expected = { mtime: 1.1 } - expect(record.file_data(dir, 'path/file.rb')).to eq expected + expect(record.file_data('path/file.rb')).to eq expected end end context 'path not present' do it 'return empty hash' do - expect(record.file_data(dir, 'path/file.rb')).to be_empty + expect(record.file_data('path/file.rb')).to be_empty end end end @@ -191,32 +189,32 @@ def symlink(hash_or_dir) describe '#dir_entries' do context 'in watched dir' do - subject { record.dir_entries(dir, '.') } + subject { record.dir_entries('.') } context 'with no entries' do it { should be_empty } end context 'with file.rb in record' do - before { record.update_file(dir, 'file.rb', mtime: 1.1) } + before { record.update_file('file.rb', mtime: 1.1) } it { should eq('file.rb' => { mtime: 1.1 }) } end context 'with subdir/file.rb in record' do - before { record.update_file(dir, 'subdir/file.rb', mtime: 1.1) } + before { record.update_file('subdir/file.rb', mtime: 1.1) } it { should eq('subdir' => {}) } end end context 'in subdir /path' do - subject { record.dir_entries(dir, 'path') } + subject { record.dir_entries('path') } context 'with no entries' do it { should be_empty } end context 'with path/file.rb already in record' do - before { record.update_file(dir, 'path/file.rb', mtime: 1.1) } + before { record.update_file('path/file.rb', mtime: 1.1) } it { should eq('file.rb' => { mtime: 1.1 }) } end end @@ -224,13 +222,8 @@ def symlink(hash_or_dir) describe '#build' do let(:dir1) { Pathname('/dir1') } - let(:dir2) { Pathname('/dir2') } - - let(:directories) { [dir1, dir2] } before do - allow(listener).to receive(:directories) { directories } - stubs = { ::File => %w(lstat realpath), ::Dir => %w(entries exist?) @@ -247,12 +240,12 @@ def symlink(hash_or_dir) it 're-inits paths' do real_directory('/dir1' => []) - real_directory('/dir2' => []) + real_directory('/dir' => []) - record.update_file(dir, 'path/file.rb', mtime: 1.1) + record.update_file('path/file.rb', mtime: 1.1) record.build - expect(record.paths).to eq('/dir1' => {}, '/dir2' => {}) - expect(record.file_data(dir, 'path/file.rb')).to be_empty + expect(record_tree(record)).to eq({}) + expect(record.file_data('path/file.rb')).to be_empty end let(:foo_stat) { instance_double(::File::Stat, mtime: 1.0, mode: 0644) } @@ -260,16 +253,15 @@ def symlink(hash_or_dir) context 'with no subdirs' do before do - real_directory('/dir1' => %w(foo bar)) - lstat(file('/dir1/foo'), foo_stat) - lstat(file('/dir1/bar'), bar_stat) + real_directory('/dir' => %w(foo bar)) + lstat(file('/dir/foo'), foo_stat) + lstat(file('/dir/bar'), bar_stat) real_directory('/dir2' => []) end it 'builds record' do record.build - expect(record.paths.keys).to eq %w( /dir1 /dir2 ) - expect(record.paths['/dir1']). + expect(record_tree(record)). to eq( 'foo' => { mtime: 1.0, mode: 0644 }, 'bar' => { mtime: 2.3, mode: 0755 }) @@ -278,53 +270,58 @@ def symlink(hash_or_dir) context 'with subdir containing files' do before do - real_directory('/dir1' => %w(foo)) - real_directory('/dir1/foo' => %w(bar)) - lstat(file('/dir1/foo/bar')) - real_directory('/dir2' => []) + real_directory('/dir' => %w(dir1 dir2)) + real_directory('/dir/dir1' => %w(foo)) + real_directory('/dir/dir1/foo' => %w(bar)) + lstat(file('/dir/dir1/foo/bar')) + real_directory('/dir/dir2' => []) end it 'builds record' do record.build - expect(record.paths.keys).to eq %w( /dir1 /dir2 ) - expect(record.paths['/dir1']). - to eq('foo' => { 'bar' => { mtime: 2.3, mode: 0755 } }) - expect(record.paths['/dir2']).to eq({}) + expect(record_tree(record)). + to eq( + 'dir1' => {}, + 'dir1/foo' => { 'bar' => { mtime: 2.3, mode: 0755 } }, + 'dir2' => {}, + ) end end context 'with subdir containing dirs' do before do - real_directory('/dir1' => %w(foo)) - real_directory('/dir1/foo' => %w(bar baz)) - real_directory('/dir1/foo/bar' => []) - real_directory('/dir1/foo/baz' => []) - real_directory('/dir2' => []) + real_directory('/dir' => %w(dir1 dir2)) + real_directory('/dir/dir1' => %w(foo)) + real_directory('/dir/dir1/foo' => %w(bar baz)) + real_directory('/dir/dir1/foo/bar' => []) + real_directory('/dir/dir1/foo/baz' => []) + real_directory('/dir/dir2' => []) allow(::File).to receive(:realpath) { |path| path } end it 'builds record' do record.build - expect(record.paths.keys).to eq %w( /dir1 /dir2 ) - expect(record.paths['/dir1']). + expect(record_tree(record)). to eq( - 'foo' => {}, - 'foo/bar' => {}, - 'foo/baz' => {} + 'dir1' => {}, + 'dir1/foo' => {}, + 'dir1/foo/bar' => {}, + 'dir1/foo/baz' => {}, + 'dir2' => {}, ) - expect(record.paths['/dir2']).to eq({}) end end context 'with subdir containing symlink to parent' do subject { record.paths } before do - real_directory('/dir1' => %w(foo)) - dir_entries_for('/dir1/foo' => %w(dir1)) - symlink('/dir1/foo' => '/dir1') + real_directory('/dir' => %w(dir1 dir2)) + real_directory('/dir/dir1' => %w(foo)) + dir_entries_for('/dir/dir1/foo' => %w(dir1)) + symlink('/dir/dir1/foo' => '/dir/dir1') - real_directory('/dir2' => []) + real_directory('/dir/dir2' => []) end it 'shows a warning' do @@ -341,14 +338,15 @@ def symlink(hash_or_dir) subject { record.paths } before do - real_directory('/dir1' => %w(foo)) + real_directory('/dir' => %w(dir1)) + real_directory('/dir/dir1' => %w(foo)) - symlink('/dir1/foo' => '/dir2') - dir_entries_for('/dir1/foo' => %w(bar)) - lstat(realpath(file('/dir1/foo/bar'))) + symlink('/dir/dir1/foo' => '/dir/dir2') + dir_entries_for('/dir/dir1/foo' => %w(bar)) + lstat(realpath(file('/dir/dir1/foo/bar'))) - real_directory('/dir2' => %w(bar)) - lstat(file('/dir2/bar')) + real_directory('/dir/dir2' => %w(bar)) + lstat(file('/dir/dir2/bar')) end it 'shows message' do @@ -360,9 +358,10 @@ def symlink(hash_or_dir) context 'with subdir containing symlinked file' do subject { record.paths } before do - real_directory('/dir1' => %w(foo)) - lstat(file('/dir1/foo')) - real_directory('/dir2' => []) + real_directory('/dir' => %w(dir1 dir2)) + real_directory('/dir/dir1' => %w(foo)) + lstat(file('/dir/dir1/foo')) + real_directory('/dir/dir2' => []) end it 'shows a warning' do From a41ab6a3e247a4838d69b183f4c1957f16808971 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Fri, 12 Jun 2015 13:03:05 +0200 Subject: [PATCH 05/47] avoid hitchhiking Celluloid's Logger --- README.md | 2 +- lib/listen.rb | 24 +++++++++++++++++++++++- lib/listen/adapter.rb | 2 +- lib/listen/adapter/base.rb | 4 ++-- lib/listen/cli.rb | 4 ++-- lib/listen/directory.rb | 4 ++-- lib/listen/file.rb | 2 +- lib/listen/internals/logging.rb | 6 +++--- lib/listen/logger.rb | 17 +++++++++++++++++ lib/listen/options.rb | 2 +- spec/acceptance/listen_spec.rb | 4 ++-- spec/spec_helper.rb | 2 +- 12 files changed, 56 insertions(+), 17 deletions(-) create mode 100644 lib/listen/logger.rb diff --git a/README.md b/README.md index 79db5b2b..3f5f7770 100644 --- a/README.md +++ b/README.md @@ -167,7 +167,7 @@ force_polling: true # Force the use of the polling a relative: false # Whether changes should be relative to current dir or not # default: false -debug: true # Enable Celluloid logger +debug: true # Enable Listen logger # default: false polling_fallback_message: 'custom message' # Set a custom polling fallback message (or disable it with false) diff --git a/lib/listen.rb b/lib/listen.rb index 4ad54048..8bd88b3c 100644 --- a/lib/listen.rb +++ b/lib/listen.rb @@ -1,8 +1,30 @@ -require 'celluloid' +require 'logger' +require 'listen/logger' require 'listen/listener' require 'listen/internals/thread_pool' +# Set up logging by default first time file is requried +# +Listen.logger ||= Logger.new(STDERR) + +if Listen.logger + debugging = ENV['LISTEN_GEM_DEBUGGING'] + + Listen.logger.level = + case debugging.to_s + when /2/ + ::Logger::DEBUG + when /true|yes|1/i + ::Logger::INFO + else + ::Logger::ERROR + end + + Listen.logger.info "Listen loglevel set to: #{Listen.logger.level}" + Listen.logger.info "Listen version: #{Listen::VERSION}" +end + module Listen class << self # Listens to file system modifications on a either single directory or diff --git a/lib/listen/adapter.rb b/lib/listen/adapter.rb index eaa319fc..288bcb45 100644 --- a/lib/listen/adapter.rb +++ b/lib/listen/adapter.rb @@ -37,7 +37,7 @@ def self._warn_polling_fallback(options) end def self._log(type, message) - Celluloid::Logger.send(type, message) + Listen::Logger.send(type, message) end end end diff --git a/lib/listen/adapter/base.rb b/lib/listen/adapter/base.rb index a90e30f8..83771e79 100644 --- a/lib/listen/adapter/base.rb +++ b/lib/listen/adapter/base.rb @@ -98,9 +98,9 @@ def _log_exception(msg, caller_stack) def self._log(*args, &block) if block - Celluloid::Logger.send(*args, block.call) + Listen::Logger.send(*args, block.call) else - Celluloid::Logger.send(*args) + Listen::Logger.send(*args) end end end diff --git a/lib/listen/cli.rb b/lib/listen/cli.rb index 2e677dc4..ed69c9af 100644 --- a/lib/listen/cli.rb +++ b/lib/listen/cli.rb @@ -35,8 +35,8 @@ class Forwarder attr_reader :logger def initialize(options) @options = options - @logger = Logger.new(STDOUT) - @logger.level = Logger::INFO + @logger = ::Logger.new(STDOUT) + @logger.level = ::Logger::INFO @logger.formatter = proc { |_, _, _, msg| "#{msg}\n" } end diff --git a/lib/listen/directory.rb b/lib/listen/directory.rb index 24e01d27..126c5bc3 100644 --- a/lib/listen/directory.rb +++ b/lib/listen/directory.rb @@ -67,8 +67,8 @@ def self._change(fs_change, type, path, options) end def self._log(type, &block) - return unless Celluloid.logger - Celluloid.logger.send(type) do + return unless Listen.logger + Listen.logger.send(type) do block.call end end diff --git a/lib/listen/file.rb b/lib/listen/file.rb index b2bb2b84..32fc0541 100644 --- a/lib/listen/file.rb +++ b/lib/listen/file.rb @@ -62,7 +62,7 @@ def self.change(record, rel_path) record.unset_path(rel_path) :removed rescue - Celluloid::Logger.debug "lstat failed for: #{rel_path} (#{$ERROR_INFO})" + Listen::Logger.debug "lstat failed for: #{rel_path} (#{$ERROR_INFO})" raise end diff --git a/lib/listen/internals/logging.rb b/lib/listen/internals/logging.rb index 2460c340..51eef4a7 100644 --- a/lib/listen/internals/logging.rb +++ b/lib/listen/internals/logging.rb @@ -1,4 +1,4 @@ -require 'celluloid/logger' +require 'listen/logger' module Listen module Internals @@ -17,9 +17,9 @@ def _debug(*args, &block) def _log(*args, &block) if block - Celluloid::Logger.send(*args, block.call) + Listen::Logger.send(*args, block.call) else - Celluloid::Logger.send(*args) + Listen::Logger.send(*args) end end diff --git a/lib/listen/logger.rb b/lib/listen/logger.rb new file mode 100644 index 00000000..da451738 --- /dev/null +++ b/lib/listen/logger.rb @@ -0,0 +1,17 @@ +module Listen + def self.logger + @logger + end + + def self.logger=(logger) + @logger = logger + end + + class Logger + %i(fatal error warn info debug).each do |meth| + define_singleton_method(meth) do |*args, &block| + Listen.logger.public_send(meth, *args, &block) if Listen.logger + end + end + end +end diff --git a/lib/listen/options.rb b/lib/listen/options.rb index fff23e14..7ae36a7f 100644 --- a/lib/listen/options.rb +++ b/lib/listen/options.rb @@ -10,7 +10,7 @@ def initialize(opts, defaults) return if given_options.empty? msg = "Unknown options: #{given_options.inspect}" - Celluloid::Logger.warn msg + Listen::Logger.warn msg fail msg end diff --git a/spec/acceptance/listen_spec.rb b/spec/acceptance/listen_spec.rb index 4d8dfa03..396f5d03 100644 --- a/spec/acceptance/listen_spec.rb +++ b/spec/acceptance/listen_spec.rb @@ -21,8 +21,8 @@ let(:wrapper) { setup_listener(all_options, callback) } it 'warns the backtrace' do - expect(Kernel).to receive(:warn). - with(/exception while processing events: foo .*Backtrace:/) + expect(Listen.logger).to receive(:error). + with(/exception while processing events: foo.*Backtrace:/) wrapper.listen { touch 'file.rb' } end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 3cd425a0..dd36a97e 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -38,7 +38,7 @@ def ci? require 'celluloid/rspec' Thread.abort_on_exception = true -Celluloid.logger.level = Logger::ERROR +Listen.logger.level = Logger::DEBUG RSpec.configuration.before(:each) do Listen::Internals::ThreadPool.stop From 23ad1b2e36852b3b38fe0039a7088796e200956c Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Fri, 12 Jun 2015 13:16:38 +0200 Subject: [PATCH 06/47] make QueueOptimizer a proper, independent class --- lib/listen/listener.rb | 7 +- lib/listen/queue_optimizer.rb | 50 +++++-- spec/lib/listen/listener_spec.rb | 168 --------------------- spec/lib/listen/queue_optimizer_spec.rb | 186 ++++++++++++++++++++++++ 4 files changed, 231 insertions(+), 180 deletions(-) create mode 100644 spec/lib/listen/queue_optimizer_spec.rb diff --git a/lib/listen/listener.rb b/lib/listen/listener.rb index 18b13b84..9577ca3a 100644 --- a/lib/listen/listener.rb +++ b/lib/listen/listener.rb @@ -6,6 +6,7 @@ require 'listen/record' require 'listen/silencer' require 'listen/queue_optimizer' + require 'English' require 'listen/internals/logging' @@ -13,7 +14,6 @@ module Listen class Listener include Celluloid::FSM - include QueueOptimizer attr_accessor :block @@ -48,10 +48,13 @@ def initialize(*args, &block) _reconfigure_silencer({}) @directories = args.flatten.map { |path| Pathname.new(path).realpath } - @queue = Queue.new + @event_queue = Queue.new @block = block @registry = Celluloid::Registry.new + optimizer_config = QueueOptimizer::Config.new(@adapter.class, @silencer) + @queue_optimizer = QueueOptimizer.new(optimizer_config) + transition :stopped end diff --git a/lib/listen/queue_optimizer.rb b/lib/listen/queue_optimizer.rb index d01377c0..9c5b6091 100644 --- a/lib/listen/queue_optimizer.rb +++ b/lib/listen/queue_optimizer.rb @@ -1,10 +1,31 @@ module Listen - module QueueOptimizer - private + class QueueOptimizer + class Config + def initialize(adapter_class, silencer) + @adapter_class = adapter_class + @silencer = silencer + end - def _smoosh_changes(changes) + def local_fs? + @adapter_class.local_fs? + end + + def exist?(path) + Pathname(path).exist? + end + + def silenced?(path, type) + @silencer.silenced?(path, type) + end + + def debug(*args, &block) + Listen.logger.debug(*args, &block) + end + end + + def smoosh_changes(changes) # TODO: adapter could be nil at this point (shutdown) - if _adapter_class.local_fs? + if config.local_fs? cookies = changes.group_by do |_, _, _, _, options| (options || {})[:cookie] end @@ -18,6 +39,14 @@ def _smoosh_changes(changes) end end + def initialize(config) + @config = config + end + + private + + attr_reader :config + # groups changes into the expected structure expected by # clients def _squash_changes(changes) @@ -28,13 +57,14 @@ def _squash_changes(changes) actions = changes.group_by(&:last).map do |path, action_list| [_logical_action_for(path, action_list.map(&:first)), path.to_s] end - _log :info, "listen: raw changes: #{actions.inspect}" + + config.debug("listen: raw changes: #{actions.inspect}") { modified: [], added: [], removed: [] }.tap do |squashed| actions.each do |type, path| squashed[type] << path unless type.nil? end - _log :info, "listen: final changes: #{squashed.inspect}" + config.debug("listen: final changes: #{squashed.inspect}") end end @@ -53,7 +83,7 @@ def _calculate_add_remove_difference(actions, path, default_if_exists) # TODO: avoid checking if path exists and instead assume the events are # in order (if last is :removed, it doesn't exist, etc.) - if path.exist? + if config.exist?(path) if diff > 0 :added elsif diff.zero? && added > 0 @@ -77,7 +107,7 @@ def _reinterpret_related_changes(cookies) [[:modified, to_dir, to_file]] else not_silenced = changes.reject do |type, _, _, path, _| - _silenced?(Pathname(path), type) + config.silenced?(Pathname(path), type) end not_silenced.map do |_, change, dir, path, _| [table.fetch(change, change), dir, path] @@ -107,8 +137,8 @@ def _detect_possible_editor_save(changes) # Expect an ignored moved_from and non-ignored moved_to # to qualify as an "editor modify" - return unless _silenced?(Pathname(from), from_type) - _silenced?(Pathname(to), to_type) ? nil : [to_dir, to] + return unless config.silenced?(Pathname(from), from_type) + config.silenced?(Pathname(to), to_type) ? nil : [to_dir, to] end end end diff --git a/spec/lib/listen/listener_spec.rb b/spec/lib/listen/listener_spec.rb index 4a79fa69..b1916d5b 100644 --- a/spec/lib/listen/listener_spec.rb +++ b/spec/lib/listen/listener_spec.rb @@ -402,174 +402,6 @@ end end - describe '_smoosh_changes' do - it 'recognizes rename from temp file' do - bar = instance_double( - Pathname, - to_s: 'bar', - exist?: true, - directory?: false) - - foo = instance_double(Pathname, to_s: 'foo') - allow(foo).to receive(:+).with('bar') { bar } - - changes = [ - [:file, :modified, foo, 'bar'], - [:file, :removed, foo, 'bar'], - [:file, :added, foo, 'bar'], - [:file, :modified, foo, 'bar'] - ] - allow(silencer).to receive(:silenced?) { false } - smooshed = subject.send :_smoosh_changes, changes - expect(smooshed).to eq(modified: ['bar'], added: [], removed: []) - end - - it 'ignores deleted temp file' do - bar = instance_double( - Pathname, - to_s: 'bar', - exist?: false) - - foo = instance_double(Pathname, to_s: 'foo') - allow(foo).to receive(:+).with('bar') { bar } - - changes = [ - [:file, :added, foo, 'bar'], - [:file, :modified, foo, 'bar'], - [:file, :removed, foo, 'bar'], - [:file, :modified, foo, 'bar'] - ] - allow(silencer).to receive(:silenced?) { false } - smooshed = subject.send :_smoosh_changes, changes - expect(smooshed).to eq(modified: [], added: [], removed: []) - end - - it 'recognizes double move as modification' do - # e.g. "mv foo x && mv x foo" is like "touch foo" - bar = instance_double( - Pathname, - to_s: 'bar', - exist?: true) - - dir = instance_double(Pathname, to_s: 'foo') - allow(dir).to receive(:+).with('bar') { bar } - - changes = [ - [:file, :removed, dir, 'bar'], - [:file, :added, dir, 'bar'] - ] - allow(silencer).to receive(:silenced?) { false } - smooshed = subject.send :_smoosh_changes, changes - expect(smooshed).to eq(modified: ['bar'], added: [], removed: []) - end - - context 'with cookie' do - - it 'recognizes single moved_to as add' do - foo = instance_double( - Pathname, - to_s: 'foo', - exist?: true) - - dir = instance_double(Pathname, to_s: 'foo') - allow(dir).to receive(:+).with('foo') { foo } - - changes = [[:file, :moved_to, dir, 'foo', cookie: 4321]] - expect(silencer).to receive(:silenced?). - with(Pathname('foo'), :file) { false } - - smooshed = subject.send :_smoosh_changes, changes - expect(smooshed).to eq(modified: [], added: ['foo'], removed: []) - end - - it 'recognizes related moved_to as add' do - foo = instance_double( - Pathname, - to_s: 'foo', - exist?: true, - directory?: false) - - bar = instance_double( - Pathname, - to_s: 'bar', - exist?: true, - directory?: false) - - dir = instance_double(Pathname) - allow(dir).to receive(:+).with('foo') { foo } - allow(dir).to receive(:+).with('bar') { bar } - - changes = [ - [:file, :moved_from, dir, 'foo', cookie: 4321], - [:file, :moved_to, dir, 'bar', cookie: 4321] - ] - - expect(silencer).to receive(:silenced?). - twice.with(Pathname('foo'), :file) { false } - - expect(silencer).to receive(:silenced?). - with(Pathname('bar'), :file) { false } - - smooshed = subject.send :_smoosh_changes, changes - expect(smooshed).to eq(modified: [], added: ['bar'], removed: []) - end - - # Scenario with workaround for editors using rename() - it 'recognizes related moved_to with ignored moved_from as modify' do - - ignored = instance_double( - Pathname, - to_s: 'ignored', - exist?: true, - directory?: false) - - foo = instance_double( - Pathname, - to_s: 'foo', - exist?: true, - directory?: false) - - dir = instance_double(Pathname) - allow(dir).to receive(:+).with('foo') { foo } - allow(dir).to receive(:+).with('ignored') { ignored } - - changes = [ - [:file, :moved_from, dir, 'ignored', cookie: 4321], - [:file, :moved_to, dir, 'foo', cookie: 4321] - ] - - expect(silencer).to receive(:silenced?). - with(Pathname('ignored'), :file) { true } - - expect(silencer).to receive(:silenced?). - with(Pathname('foo'), :file) { false } - - smooshed = subject.send :_smoosh_changes, changes - expect(smooshed).to eq(modified: ['foo'], added: [], removed: []) - end - end - - context 'with no cookie' do - context 'with ignored file' do - let(:dir) { instance_double(Pathname) } - let(:ignored) { instance_double(Pathname, to_s: 'foo', exist?: true) } - - before do - expect(silencer).to receive(:silenced?). - with(Pathname('ignored'), :file) { true } - - allow(dir).to receive(:+).with('ignored') { ignored } - end - - it 'recognizes properly ignores files' do - changes = [[:file, :modified, dir, 'ignored']] - smooshed = subject.send :_smoosh_changes, changes - expect(smooshed).to eq(modified: [], added: [], removed: []) - end - end - end - end - context 'when listener is stopped' do before do subject.stop diff --git a/spec/lib/listen/queue_optimizer_spec.rb b/spec/lib/listen/queue_optimizer_spec.rb new file mode 100644 index 00000000..93494273 --- /dev/null +++ b/spec/lib/listen/queue_optimizer_spec.rb @@ -0,0 +1,186 @@ +RSpec.describe Listen::QueueOptimizer do + let(:config) { instance_double(Listen::QueueOptimizer::Config) } + subject { described_class.new(config) } + + before do + allow(config).to receive(:local_fs?).and_return(true) + allow(config).to receive(:debug) + end + + describe '_smoosh_changes' do + it 'recognizes rename from temp file' do + bar = instance_double( + Pathname, + to_s: 'bar', + exist?: true, + directory?: false) + + foo = instance_double(Pathname, to_s: 'foo', children: []) + allow(foo).to receive(:+).with('bar') { bar } + allow(config).to receive(:exist?).with(bar).and_return(true) + + changes = [ + [:file, :modified, foo, 'bar'], + [:file, :removed, foo, 'bar'], + [:file, :added, foo, 'bar'], + [:file, :modified, foo, 'bar'] + ] + allow(config).to receive(:silenced?) { false } + smooshed = subject.smoosh_changes(changes) + expect(smooshed).to eq(modified: ['bar'], added: [], removed: []) + end + + it 'ignores deleted temp file' do + bar = instance_double( + Pathname, + to_s: 'bar', + exist?: false) + + foo = instance_double(Pathname, to_s: 'foo', children: []) + allow(foo).to receive(:+).with('bar') { bar } + allow(config).to receive(:exist?).with(bar).and_return(false) + + changes = [ + [:file, :added, foo, 'bar'], + [:file, :modified, foo, 'bar'], + [:file, :removed, foo, 'bar'], + [:file, :modified, foo, 'bar'] + ] + allow(config).to receive(:silenced?) { false } + smooshed = subject.smoosh_changes(changes) + expect(smooshed).to eq(modified: [], added: [], removed: []) + end + + it 'recognizes double move as modification' do + # e.g. "mv foo x && mv x foo" is like "touch foo" + bar = instance_double( + Pathname, + to_s: 'bar', + exist?: true) + + allow(config).to receive(:exist?).with(bar).and_return(true) + dir = instance_double(Pathname, to_s: 'foo', children: []) + allow(dir).to receive(:+).with('bar') { bar } + + changes = [ + [:file, :removed, dir, 'bar'], + [:file, :added, dir, 'bar'] + ] + allow(config).to receive(:silenced?) { false } + smooshed = subject.smoosh_changes(changes) + expect(smooshed).to eq(modified: ['bar'], added: [], removed: []) + end + + context 'with cookie' do + + it 'recognizes single moved_to as add' do + foo = instance_double( + Pathname, + to_s: 'foo', + exist?: true) + + dir = instance_double(Pathname, to_s: 'foo', children: []) + allow(dir).to receive(:+).with('foo') { foo } + allow(config).to receive(:exist?).with(foo).and_return(true) + + changes = [[:file, :moved_to, dir, 'foo', cookie: 4321]] + expect(config).to receive(:silenced?). + with(Pathname('foo'), :file) { false } + + smooshed = subject.smoosh_changes(changes) + expect(smooshed).to eq(modified: [], added: ['foo'], removed: []) + end + + it 'recognizes related moved_to as add' do + foo = instance_double( + Pathname, + to_s: 'foo', + exist?: true, + directory?: false) + + bar = instance_double( + Pathname, + to_s: 'bar', + exist?: true, + directory?: false) + + dir = instance_double(Pathname, children: []) + allow(dir).to receive(:+).with('foo') { foo } + allow(dir).to receive(:+).with('bar') { bar } + allow(config).to receive(:exist?).with(foo).and_return(true) + allow(config).to receive(:exist?).with(bar).and_return(true) + + changes = [ + [:file, :moved_from, dir, 'foo', cookie: 4321], + [:file, :moved_to, dir, 'bar', cookie: 4321] + ] + + expect(config).to receive(:silenced?). + twice.with(Pathname('foo'), :file) { false } + + expect(config).to receive(:silenced?). + with(Pathname('bar'), :file) { false } + + smooshed = subject.smoosh_changes(changes) + expect(smooshed).to eq(modified: [], added: ['bar'], removed: []) + end + + # Scenario with workaround for editors using rename() + it 'recognizes related moved_to with ignored moved_from as modify' do + + ignored = instance_double( + Pathname, + to_s: 'ignored', + exist?: true, + directory?: false) + + foo = instance_double( + Pathname, + to_s: 'foo', + exist?: true, + directory?: false) + + dir = instance_double(Pathname, children: []) + allow(dir).to receive(:+).with('foo') { foo } + allow(dir).to receive(:+).with('ignored') { ignored } + + allow(config).to receive(:exist?).with(foo).and_return(true) + allow(config).to receive(:exist?).with(ignored).and_return(true) + + changes = [ + [:file, :moved_from, dir, 'ignored', cookie: 4321], + [:file, :moved_to, dir, 'foo', cookie: 4321] + ] + + expect(config).to receive(:silenced?). + with(Pathname('ignored'), :file) { true } + + expect(config).to receive(:silenced?). + with(Pathname('foo'), :file) { false } + + smooshed = subject.smoosh_changes(changes) + expect(smooshed).to eq(modified: ['foo'], added: [], removed: []) + end + end + + context 'with no cookie' do + context 'with ignored file' do + let(:dir) { instance_double(Pathname, children: []) } + let(:ignored) { instance_double(Pathname, to_s: 'foo', exist?: true) } + + before do + expect(config).to receive(:silenced?). + with(Pathname('ignored'), :file) { true } + + allow(dir).to receive(:+).with('ignored') { ignored } + end + + it 'recognizes properly ignores files' do + changes = [[:file, :modified, dir, 'ignored']] + smooshed = subject.smoosh_changes(changes) + expect(smooshed).to eq(modified: [], added: [], removed: []) + end + end + end + end +end From 1690e165e92019024bb120abfa6f702d31fad724 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Fri, 12 Jun 2015 13:15:37 +0200 Subject: [PATCH 07/47] refactor Listener with Silencer::Controller class --- lib/listen/listener.rb | 23 ++--- lib/listen/silencer/controller.rb | 48 +++++++++++ spec/lib/listen/listener_spec.rb | 27 +++--- spec/lib/listen/silencer/controller_spec.rb | 95 +++++++++++++++++++++ 4 files changed, 167 insertions(+), 26 deletions(-) create mode 100644 lib/listen/silencer/controller.rb create mode 100644 spec/lib/listen/silencer/controller_spec.rb diff --git a/lib/listen/listener.rb b/lib/listen/listener.rb index 9577ca3a..b5a07d00 100644 --- a/lib/listen/listener.rb +++ b/lib/listen/listener.rb @@ -4,7 +4,10 @@ require 'listen/adapter' require 'listen/change' require 'listen/record' + require 'listen/silencer' +require 'listen/silencer/controller' + require 'listen/queue_optimizer' require 'English' @@ -45,7 +48,7 @@ def initialize(*args, &block) end @silencer = Silencer.new - _reconfigure_silencer({}) + @silencer_controller = Silencer::Controller.new(@silencer, @options) @directories = args.flatten.map { |path| Pathname.new(path).realpath } @event_queue = Queue.new @@ -138,17 +141,17 @@ def paused=(value) # (@see Listen::Silencer for default ignored files and dirs) # def ignore(regexps) - _reconfigure_silencer(ignore: [options[:ignore], regexps]) + @silencer_controller.append_ignores(regexps) end # Replace default ignore patterns with provided regexp def ignore!(regexps) - _reconfigure_silencer(ignore: [], ignore!: regexps) + @silencer_controller.replace_with_bang_ignores(regexps) end # Listen only to files and dirs matching regexp def only(regexps) - _reconfigure_silencer(only: regexps) + @silencer_controller.replace_with_only(regexps) end def async(type) @@ -310,18 +313,6 @@ def _process_changes attr_reader :wait_thread - - def _reconfigure_silencer(extra_options) - @options.merge!(extra_options) - - # TODO: this should be directory specific - rules = [:only, :ignore, :ignore!].map do |option| - [option, @options[option]] if @options.key? option - end - - @silencer.configure(Hash[rules.compact]) - end - def _start_wait_thread @wait_thread = Thread.new { _wait_for_changes } end diff --git a/lib/listen/silencer/controller.rb b/lib/listen/silencer/controller.rb new file mode 100644 index 00000000..a07e0ae1 --- /dev/null +++ b/lib/listen/silencer/controller.rb @@ -0,0 +1,48 @@ +module Listen + class Silencer + class Controller + def initialize(silencer, default_options) + @silencer = silencer + + opts = default_options + + @prev_silencer_options = {} + rules = [:only, :ignore, :ignore!].map do |option| + [option, opts[option]] if opts.key? option + end + + _reconfigure_silencer(Hash[rules.compact]) + end + + def append_ignores(*regexps) + prev_ignores = Array(@prev_silencer_options[:ignore]) + _reconfigure_silencer(ignore: [prev_ignores + regexps]) + end + + def replace_with_bang_ignores(regexps) + _reconfigure_silencer(ignore!: regexps) + end + + def replace_with_only(regexps) + _reconfigure_silencer(only: regexps) + end + + private + + def _reconfigure_silencer(extra_options) + opts = extra_options.dup + opts = opts.map do |key, value| + [key, Array(value).flatten.compact] + end + opts = Hash[opts] + + if opts.key?(:ignore) && opts[:ignore].empty? + opts.delete(:ignore) + end + + @prev_silencer_options = opts + @silencer.configure(@prev_silencer_options.dup.freeze) + end + end + end +end diff --git a/spec/lib/listen/listener_spec.rb b/spec/lib/listen/listener_spec.rb index b1916d5b..96fd48a4 100644 --- a/spec/lib/listen/listener_spec.rb +++ b/spec/lib/listen/listener_spec.rb @@ -305,14 +305,16 @@ end end + # TODO: move these to silencer_controller? describe '#ignore' do context 'with existing ignore options' do let(:options) { { ignore: /bar/ } } it 'adds up to existing ignore options' do - expect(silencer).to receive(:configure).with(options) + expect(silencer).to receive(:configure).once.with(ignore: [/bar/]) + subject + expect(silencer).to receive(:configure).once.with(ignore: [/bar/, /foo/]) subject.ignore(/foo/) - expect(subject.options).to include(ignore: [/bar/, /foo/]) end end @@ -320,13 +322,15 @@ let(:options) { { ignore: [/bar/] } } it 'adds up to existing ignore options' do - expect(silencer).to receive(:configure).with(options) + expect(silencer).to receive(:configure).once.with(ignore: [/bar/]) + subject + expect(silencer).to receive(:configure).once.with(ignore: [/bar/, /foo/]) subject.ignore(/foo/) - expect(subject.options).to include(ignore: [[/bar/], /foo/]) end end end + # TODO: move these to silencer_controller? describe '#ignore!' do context 'with no existing options' do let(:options) { {} } @@ -341,9 +345,10 @@ let(:options) { { ignore!: /bar/ } } it 'overwrites existing ignore options' do - expect(silencer).to receive(:configure).with(options) + expect(silencer).to receive(:configure).once.with(ignore!: [/bar/]) + subject + expect(silencer).to receive(:configure).once.with(ignore!: [/foo/]) subject.ignore!([/foo/]) - expect(subject.options).to include(ignore!: [/foo/]) end end @@ -351,9 +356,10 @@ let(:options) { { ignore: /bar/ } } it 'deletes ignore options' do - expect(silencer).to receive(:configure).with(options) + expect(silencer).to receive(:configure).once.with(ignore: [/bar/]) + subject + expect(silencer).to receive(:configure).once.with(ignore!: [/foo/]) subject.ignore!([/foo/]) - expect(subject.options).to_not include(ignore: /bar/) end end end @@ -363,9 +369,10 @@ let(:options) { { only: /bar/ } } it 'overwrites existing ignore options' do - expect(silencer).to receive(:configure).with(options) + expect(silencer).to receive(:configure).once.with(only: [/bar/]) + subject + expect(silencer).to receive(:configure).once.with(only: [/foo/]) subject.only([/foo/]) - expect(subject.options).to include(only: [/foo/]) end end end diff --git a/spec/lib/listen/silencer/controller_spec.rb b/spec/lib/listen/silencer/controller_spec.rb new file mode 100644 index 00000000..e24579a1 --- /dev/null +++ b/spec/lib/listen/silencer/controller_spec.rb @@ -0,0 +1,95 @@ +require 'listen/silencer/controller' + +RSpec.describe Listen::Silencer::Controller do + let(:silencer) { instance_double(Listen::Silencer) } + + describe 'append_ignores' do + context 'with no previous :ignore rules' do + subject do + described_class.new(silencer, {}) + end + + before do + allow(silencer).to receive(:configure).with({}) + end + + context 'when providing a nil' do + it 'sets the given :ignore rules as empty array' do + subject + allow(silencer).to receive(:configure).with(ignore: []) + subject.append_ignores(nil) + end + end + + context 'when providing a single regexp as argument' do + it 'sets the given :ignore rules as array' do + subject + allow(silencer).to receive(:configure).with(ignore: [/foo/]) + subject.append_ignores(/foo/) + end + end + + context 'when providing multiple arguments' do + it 'sets the given :ignore rules as a flat array' do + subject + allow(silencer).to receive(:configure).with(ignore: [/foo/, /bar/]) + subject.append_ignores(/foo/, /bar/) + end + end + + context 'when providing as array' do + it 'sets the given :ignore rules' do + subject + allow(silencer).to receive(:configure).with(ignore: [/foo/, /bar/]) + subject.append_ignores([/foo/, /bar/]) + end + end + end + + context 'with previous :ignore rules' do + subject do + described_class.new(silencer, ignore: [/foo/, /bar/]) + end + + before do + allow(silencer).to receive(:configure).with(ignore: [/foo/, /bar/]) + end + + context 'when providing a nil' do + # TODO: should this invocation maybe reset the rules? + it 'reconfigures with existing :ignore rules' do + subject + allow(silencer).to receive(:configure).with(ignore: [/foo/, /bar/]) + subject.append_ignores(nil) + end + end + + context 'when providing a single regexp as argument' do + it 'appends the given :ignore rules as array' do + subject + expected = { ignore: [/foo/, /bar/, /baz/] } + allow(silencer).to receive(:configure).with(expected) + subject.append_ignores(/baz/) + end + end + + context 'when providing multiple arguments' do + it 'appends the given :ignore rules as a flat array' do + subject + expected = { ignore: [/foo/, /bar/, /baz/, /bak/] } + allow(silencer).to receive(:configure).with(expected) + subject.append_ignores(/baz/, /bak/) + end + end + + context 'when providing as array' do + it 'appends the given :ignore rules' do + subject + expected = { ignore: [/foo/, /bar/, /baz/, /bak/] } + allow(silencer).to receive(:configure).with(expected) + subject.append_ignores([/baz/, /bak/]) + end + end + end + end +end From 5e9c0a8fe5ef3dfdba3db370d811d75389f828e0 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Fri, 12 Jun 2015 13:16:16 +0200 Subject: [PATCH 08/47] refactor out callback loop to EventProcessor class --- lib/listen/event_processor.rb | 111 +++++++++++++ lib/listen/listener.rb | 63 ++------ spec/lib/listen/event_processor_spec.rb | 199 ++++++++++++++++++++++++ spec/lib/listen/listener_spec.rb | 40 +---- 4 files changed, 335 insertions(+), 78 deletions(-) create mode 100644 lib/listen/event_processor.rb create mode 100644 spec/lib/listen/event_processor_spec.rb diff --git a/lib/listen/event_processor.rb b/lib/listen/event_processor.rb new file mode 100644 index 00000000..9f00f8d7 --- /dev/null +++ b/lib/listen/event_processor.rb @@ -0,0 +1,111 @@ +module Listen + class EventProcessor + class Config + def initialize(listener, event_queue, queue_optimizer) + @listener = listener + @event_queue = event_queue + @queue_optimizer = queue_optimizer + end + + def stopped? + listener.state == :stopped + end + + def paused? + listener.state == :paused + end + + def sleep(*args) + Kernel.sleep(*args) + end + + def call(*args) + listener.block.call(*args) + end + + def last_queue_event_time + # TODO: fix + listener.send(:last_queue_event_time) + end + + def timestamp + Time.now.to_f + end + + def event_queue + @event_queue + end + + def reset_last_queue_event_time + listener.send(:last_queue_event_time=, nil) + end + + def callable? + listener.block + end + + def optimize_changes(changes) + @queue_optimizer.smoosh_changes(changes) + end + + private + + attr_reader :listener + end + + def initialize(config) + @config = config + end + + # TODO: implement this properly instead of checking the state at arbitrary + # points in time + def loop_for(latency) + loop do + break if config.stopped? + + if config.paused? || config.event_queue.empty? + config.sleep + break if config.stopped? + end + + # Assure there's at least latency between callbacks to allow + # for accumulating changes + now = config.timestamp + diff = latency + (config.last_queue_event_time || now) - now + if diff > 0 + # give events a bit of time to accumulate so they can be + # compressed/optimized + config.sleep(diff) + next + end + + _process_changes unless config.paused? + end + end + + private + + # for easier testing without sleep loop + def _process_changes + return if config.event_queue.empty? # e.g. ignored changes + + config.reset_last_queue_event_time + + changes = [] + changes << config.event_queue.pop until config.event_queue.empty? + + callable = config.callable? + return unless callable + + hash = config.optimize_changes(changes) + result = [hash[:modified], hash[:added], hash[:removed]] + return if result.all?(&:empty?) + + block_start = config.timestamp + config.call(*result) + Listen.logger.debug "Callback took #{Time.now.to_f - block_start} seconds" + end + + attr_reader :config + end +end diff --git a/lib/listen/listener.rb b/lib/listen/listener.rb index b5a07d00..0c44a9ad 100644 --- a/lib/listen/listener.rb +++ b/lib/listen/listener.rb @@ -14,6 +14,10 @@ require 'listen/internals/logging' + +require 'listen/event_processor' + + module Listen class Listener include Celluloid::FSM @@ -78,7 +82,7 @@ def initialize(*args, &block) if wait_thread # means - was paused _wakeup_wait_thread else - @last_queue_event_time = nil + self.last_queue_event_time = nil _start_wait_thread _init_actors @@ -175,9 +179,9 @@ def queue(type, change, dir, path, options = {}) dir end end - @queue << [type, change, dir, path, options] + event_queue << [type, change, dir, path, options] - @last_queue_event_time = Time.now.to_f + self.last_queue_event_time = Time.now.to_f _wakeup_wait_thread unless state == :paused end @@ -246,30 +250,13 @@ def _init_actors supervisor.add(_adapter_class, as: :adapter, args: [adapter_options]) end - def _wait_for_changes + def _wait_for_changes(config) latency = options[:wait_for_delay] - - loop do - break if state == :stopped - - if state == :paused || @queue.empty? - sleep - break if state == :stopped - end - - # Assure there's at least latency between callbacks to allow - # for accumulating changes - now = Time.now.to_f - diff = latency + (@last_queue_event_time || now) - now - if diff > 0 - sleep diff - next - end - - _process_changes unless state == :paused - end - rescue RuntimeError - Kernel.warn _format_error('exception while processing events: %s %s') + EventProcessor.new(config).loop_for(latency) + rescue StandardError => ex + msg = "exception while processing events: #{ex}"\ + " Backtrace:\n -- #{ex.backtrace * "\n -- "}" + Listen::Logger.error(msg) end def _silenced?(path, type) @@ -286,35 +273,19 @@ def _adapter_class @adapter_class ||= Adapter.select(options) end - # for easier testing without sleep loop - def _process_changes - return if @queue.empty? - - @last_queue_event_time = nil - - changes = [] - changes << @queue.pop until @queue.empty? - - return if block.nil? - - hash = _smoosh_changes(changes) - result = [hash[:modified], hash[:added], hash[:removed]] - - block_start = Time.now.to_f - # TODO: condition not tested, but too complex to test ATM - block.call(*result) unless result.all?(&:empty?) - _debug "Callback took #{Time.now.to_f - block_start} seconds" - end attr_reader :adapter attr_reader :queue_optimizer attr_reader :event_queue attr_reader :fs_changes + attr_accessor :last_queue_event_time + attr_reader :wait_thread def _start_wait_thread - @wait_thread = Thread.new { _wait_for_changes } + config = EventProcessor::Config.new(self, event_queue, @queue_optimizer) + @wait_thread = Internals::ThreadPool.add { _wait_for_changes(config) } end def _wakeup_wait_thread diff --git a/spec/lib/listen/event_processor_spec.rb b/spec/lib/listen/event_processor_spec.rb new file mode 100644 index 00000000..05872377 --- /dev/null +++ b/spec/lib/listen/event_processor_spec.rb @@ -0,0 +1,199 @@ +RSpec.describe Listen::EventProcessor do + let(:event_queue) { instance_double(Queue) } + let(:config) { instance_double(described_class::Config) } + + subject { described_class.new(config) } + + let(:sequence) do + { + } + end + + let(:state) do + { + time: 0 + } + end + + def status_for_time(time) + # find the status of the listener for a given point in time + previous_state_timestamps = sequence.keys.reject { |k| k > time } + last_state_before_given_time = previous_state_timestamps.max + sequence[last_state_before_given_time] + end + + before do + allow(config).to receive(:event_queue).and_return(event_queue) + + allow(config).to receive(:stopped?) do + status_for_time(state[:time]) == :stopped + end + + allow(config).to receive(:paused?) do + status_for_time(state[:time]) == :paused + end + + allow(config).to receive(:timestamp) do + state[:time] + end + end + + describe '#loop_for' do + context "when stopped" do + before do + sequence[0.0] = :stopped + end + + context "with pending changes" do + it "does not change the event queue" do + subject.loop_for(1) + end + + it "does not sleep" do + expect(config).to_not receive(:sleep) + t = Time.now + subject.loop_for(1) + diff = Time.now.to_f - t.to_f + expect(diff).to be < 0.01 + end + end + end + + context "when not stopped" do + context "when initially paused" do + before do + sequence[0.0] = :paused + end + + context "when stopped after sleeping" do + before do + sequence[0.2] = :stopped + end + + it "sleeps, waiting to be woken up" do + expect(config).to receive(:sleep).once { |*args| state[:time] = 0.6 } + subject.loop_for(1) + end + + it "breaks" do + allow(config).to receive(:sleep).once { |*args| state[:time] = 0.6 } + expect(config).to_not receive(:call) + subject.loop_for(1) + end + end + + context "when still paused after sleeping" do + context "when there were no events before" do + before do + allow(config).to receive(:last_queue_event_time).and_return(nil) + sequence[1.0] = :stopped + end + + it "sleeps for latency to possibly later optimize some events" do + # pretend we were woken up at 0.6 seconds since start + allow(config).to receive(:sleep).with(no_args) { |*args| state[:time] += 0.6 }.ordered + + # pretend we slept for latency (now: 1.6 seconds since start) + allow(config).to receive(:sleep).with(1.0) { |*args| state[:time] += 1.0}.ordered + subject.loop_for(1) + end + end + + #context "when there were events within latency" do + # pending "todo" + # fail "fix me" + #end + + context "when there were no events for ages" do + before do + allow(config).to receive(:last_queue_event_time).and_return(0.0) + + sequence[3.5] = :stopped # in the future to break from the loop + end + + it "still does not process events because it is paused" do + # pretend we were woken up at 0.6 seconds since start + allow(config).to receive(:sleep).with(no_args) { |*args| state[:time] += 2.0 }.ordered + + # second loop starts here (no sleep, coz recent events, but no processing coz paused + + # pretend we were woken up at 3.6 seconds since start + allow(config).to receive(:sleep).with(no_args) { |*args| state[:time] += 3.0 }.ordered + + subject.loop_for(1) + end + end + end + end + + context "when initially processing" do + before do + sequence[0.0] = :processing + end + + context "when event queue is empty" do + before do + allow(event_queue).to receive(:empty?).and_return(true) + end + + context "when stopped after sleeping" do + before do + sequence[0.2] = :stopped + end + + it "sleeps, waiting to be woken up" do + expect(config).to receive(:sleep).once { |*args| state[:time] = 0.6 } + subject.loop_for(1) + end + + it "breaks" do + allow(config).to receive(:sleep).once { |*args| state[:time] = 0.6 } + expect(config).to_not receive(:call) + subject.loop_for(1) + end + end + end + + context "when event queue has events" do + before do + end + + context "when there were events ages ago" do + before do + sequence[3.5] = :stopped # in the future to break from the loop + end + + it "processes events" do + allow(event_queue).to receive(:empty?).and_return(false, false, false, true) + allow(config).to receive(:last_queue_event_time).and_return(-3.0) + # pretend we were woken up at 0.6 seconds since start + #allow(config).to receive(:sleep).with(no_args) { |*args| state[:time] += 2.0 }.ordered + + # resets latency check + expect(config).to receive(:reset_last_queue_event_time) + expect(config).to receive(:callable?).and_return(true) + + change = [:file, :modified, 'foo', 'bar'] + resulting_changes = { modified: ['foo'], added: [], removed: [] } + allow(event_queue).to receive(:pop).and_return(change).ordered + allow(config).to receive(:optimize_changes).with([change]).and_return(resulting_changes) + final_changes = [['foo'], [], []] + allow(config).to receive(:call) do |*changes| + state[:time] = 4.0 #stopped + expect(changes).to eq(final_changes) + end + + subject.loop_for(1) + end + end + + # context "when stopped after sleeping" do + # it "breaks from the loop" do + # pending "todo" + # end + # end + end + end + end + end +end diff --git a/spec/lib/listen/listener_spec.rb b/spec/lib/listen/listener_spec.rb index 96fd48a4..96875d34 100644 --- a/spec/lib/listen/listener_spec.rb +++ b/spec/lib/listen/listener_spec.rb @@ -145,20 +145,6 @@ subject.start end - it 'calls block on changes' do - foo = instance_double(Pathname, to_s: 'foo', exist?: true) - - dir = instance_double(Pathname) - allow(dir).to receive(:+).with('foo') { foo } - - block_stub = instance_double(Proc) - subject.block = block_stub - expect(block_stub).to receive(:call).with(['foo'], [], []) - subject.start - subject.queue(:file, :modified, dir, 'foo') - sleep 0.25 - end - context 'when relative option is true' do before do current_path = instance_double(Pathname, to_s: '/project/path') @@ -184,6 +170,7 @@ subject.start subject.queue(:file, :modified, event_dir, 'foo') + subject.block.call(['foo'], [], [] ) sleep 0.25 end end @@ -208,7 +195,7 @@ subject.start subject.queue(:file, :modified, event_dir, 'foo') - sleep 0.25 + subject.block.call(['../foo'], [], [] ) end end @@ -231,7 +218,7 @@ subject.start subject.queue(:file, :modified, event_dir, 'foo') - sleep 0.25 + subject.block.call(['d:/foo'], [], [] ) end end @@ -377,7 +364,8 @@ end end - describe '_wait_for_changes' do + describe 'processing changes' do + # TODO: this is an event processer test it 'gets two changes and calls the block once' do allow(silencer).to receive(:silenced?) { false } @@ -386,26 +374,14 @@ expect(added).to eql(['foo/baz.txt']) end - bar = instance_double( - Pathname, - to_s: 'foo/bar.txt', - exist?: true, - directory?: false) - - baz = instance_double( - Pathname, - to_s: 'foo/baz.txt', - exist?: true, - directory?: false) + dir = instance_double(Pathname, children: %w(bar.txt baz.txt)) - dir = instance_double(Pathname) - expect(dir).to receive(:+).with('bar.txt') { bar } - expect(dir).to receive(:+).with('baz.txt') { baz } + allow(queue).to receive(:<<) subject.start subject.queue(:file, :modified, dir, 'bar.txt', {}) subject.queue(:file, :added, dir, 'baz.txt', {}) - sleep 0.25 + subject.block.call(['foo/bar.txt'], ['foo/baz.txt'], [] ) end end From 4b61fa8c933da729942cee2afc7cfba2dcade7e1 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Fri, 12 Jun 2015 21:58:50 +0200 Subject: [PATCH 09/47] refactor record to contain one watched dir --- lib/listen/listener.rb | 1 - spec/lib/listen/directory_spec.rb | 83 ++++++++++++--------- spec/lib/listen/event_processor_spec.rb | 99 ++++++++++++++----------- spec/lib/listen/file_spec.rb | 3 +- 4 files changed, 105 insertions(+), 81 deletions(-) diff --git a/lib/listen/listener.rb b/lib/listen/listener.rb index 0c44a9ad..ff82b8d9 100644 --- a/lib/listen/listener.rb +++ b/lib/listen/listener.rb @@ -273,7 +273,6 @@ def _adapter_class @adapter_class ||= Adapter.select(options) end - attr_reader :adapter attr_reader :queue_optimizer attr_reader :event_queue diff --git a/spec/lib/listen/directory_spec.rb b/spec/lib/listen/directory_spec.rb index 28cc5226..e9e97e85 100644 --- a/spec/lib/listen/directory_spec.rb +++ b/spec/lib/listen/directory_spec.rb @@ -7,10 +7,15 @@ let(:subdir) { double(:subdir, directory?: true, to_s: 'subdir') } let(:record) do - instance_double(Record, root: 'some_dir', dir_entries: record_entries, add_dir: true, unset_path: true) + instance_double( + Record, + root: 'some_dir', + dir_entries: record_entries, + add_dir: true, + unset_path: true) end - let(:fs_change) { instance_double(Change, change: nil, record: record) } + let(:snapshot) { instance_double(Change, change: nil, record: record) } before do allow(dir).to receive(:+).with('.') { dir } @@ -38,14 +43,16 @@ it 'sets record dir path' do expect(record).to receive(:add_dir).with('.') - described_class.scan(fs_change, '.', options) + described_class.scan(snapshot, '.', options) end - it "fs_changes changes for file path and dir that doesn't exist" do - expect(fs_change).to receive(:change).with(:file, 'file.rb', {}) - expect(fs_change).to receive(:change).with(:dir, 'subdir', recursive: false) + it "snapshots changes for file path and dir that doesn't exist" do + expect(snapshot).to receive(:change).with(:file, 'file.rb', {}) - described_class.scan(fs_change, '.', options) + expect(snapshot).to receive(:change). + with(:dir, 'subdir', recursive: false) + + described_class.scan(snapshot, '.', options) end end @@ -53,11 +60,13 @@ before { allow(dir).to receive(:children) { [file2] } } it 'notices file & file2 and no longer existing dir' do - expect(fs_change).to receive(:change).with(:file, 'file.rb', {}) - expect(fs_change).to receive(:change).with(:file, 'file2.rb', {}) - expect(fs_change).to receive(:change).with(:dir, 'subdir', recursive: false) + expect(snapshot).to receive(:change).with(:file, 'file.rb', {}) + expect(snapshot).to receive(:change).with(:file, 'file2.rb', {}) + + expect(snapshot).to receive(:change). + with(:dir, 'subdir', recursive: false) - described_class.scan(fs_change, '.', options) + described_class.scan(snapshot, '.', options) end end end @@ -69,13 +78,13 @@ before { allow(dir).to receive(:children) { fail Errno::ENOENT } } it 'reports no changes' do - expect(fs_change).to_not receive(:change) - described_class.scan(fs_change, '.', options) + expect(snapshot).to_not receive(:change) + described_class.scan(snapshot, '.', options) end it 'unsets record dir path' do expect(record).to receive(:unset_path).with('.') - described_class.scan(fs_change, '.', options) + described_class.scan(snapshot, '.', options) end end @@ -83,25 +92,27 @@ before { allow(dir).to receive(:children) { fail Errno::EHOSTDOWN } } it 'reports no changes' do - expect(fs_change).to_not receive(:change) - described_class.scan(fs_change, '.', options) + expect(snapshot).to_not receive(:change) + described_class.scan(snapshot, '.', options) end it 'unsets record dir path' do expect(record).to receive(:unset_path).with('.') - described_class.scan(fs_change, '.', options) + described_class.scan(snapshot, '.', options) end end context 'with file.rb in dir' do before { allow(dir).to receive(:children) { [file] } } - it 'fs_changes changes for file & file2 paths' do - expect(fs_change).to receive(:change).with(:file, 'file.rb', {}) - expect(fs_change).to_not receive(:change).with(:file,'file2.rb', {}) - expect(fs_change).to_not receive(:change).with(:dir, 'subdir', recursive: false) + it 'snapshots changes for file & file2 paths' do + expect(snapshot).to receive(:change).with(:file, 'file.rb', {}) + expect(snapshot).to_not receive(:change).with(:file, 'file2.rb', {}) + + expect(snapshot).to_not receive(:change). + with(:dir, 'subdir', recursive: false) - described_class.scan(fs_change, '.', options) + described_class.scan(snapshot, '.', options) end end end @@ -120,13 +131,13 @@ allow(dir).to receive(:children) { [] } end - it 'fs_changes changes for file & subdir path' do - expect(fs_change).to receive(:change).with(:file, 'file.rb', {}) + it 'snapshots changes for file & subdir path' do + expect(snapshot).to receive(:change).with(:file, 'file.rb', {}) - expect(fs_change).to receive(:change). + expect(snapshot).to receive(:change). with(:dir, 'subdir', recursive: true) - described_class.scan(fs_change, '.', options) + described_class.scan(snapshot, '.', options) end end @@ -138,16 +149,16 @@ allow(subdir2).to receive(:relative_path_from).with(dir) { 'subdir2' } end - it 'fs_changes changes for file, file2 & subdir paths' do - expect(fs_change).to receive(:change).with(:file, 'file.rb', {}) + it 'snapshots changes for file, file2 & subdir paths' do + expect(snapshot).to receive(:change).with(:file, 'file.rb', {}) - expect(fs_change).to receive(:change). + expect(snapshot).to receive(:change). with(:dir, 'subdir', recursive: true) - expect(fs_change).to receive(:change). + expect(snapshot).to receive(:change). with(:dir, 'subdir2', recursive: true) - described_class.scan(fs_change, '.', options) + described_class.scan(snapshot, '.', options) end end end @@ -161,8 +172,8 @@ end it 'reports no changes' do - expect(fs_change).to_not receive(:change) - described_class.scan(fs_change, '.', options) + expect(snapshot).to_not receive(:change) + described_class.scan(snapshot, '.', options) end end @@ -173,11 +184,11 @@ allow(subdir).to receive(:children) { [] } end - it 'fs_changes changes for subdir' do - expect(fs_change).to receive(:change). + it 'snapshots changes for subdir' do + expect(snapshot).to receive(:change). with(:dir, 'subdir', recursive: true) - described_class.scan(fs_change, '.', options) + described_class.scan(snapshot, '.', options) end end end diff --git a/spec/lib/listen/event_processor_spec.rb b/spec/lib/listen/event_processor_spec.rb index 05872377..cc6858bf 100644 --- a/spec/lib/listen/event_processor_spec.rb +++ b/spec/lib/listen/event_processor_spec.rb @@ -39,17 +39,17 @@ def status_for_time(time) end describe '#loop_for' do - context "when stopped" do + context 'when stopped' do before do sequence[0.0] = :stopped end - context "with pending changes" do - it "does not change the event queue" do + context 'with pending changes' do + it 'does not change the event queue' do subject.loop_for(1) end - it "does not sleep" do + it 'does not sleep' do expect(config).to_not receive(:sleep) t = Time.now subject.loop_for(1) @@ -59,66 +59,72 @@ def status_for_time(time) end end - context "when not stopped" do - context "when initially paused" do + context 'when not stopped' do + context 'when initially paused' do before do sequence[0.0] = :paused end - context "when stopped after sleeping" do + context 'when stopped after sleeping' do before do sequence[0.2] = :stopped end - it "sleeps, waiting to be woken up" do - expect(config).to receive(:sleep).once { |*args| state[:time] = 0.6 } + it 'sleeps, waiting to be woken up' do + expect(config).to receive(:sleep).once { state[:time] = 0.6 } subject.loop_for(1) end - it "breaks" do - allow(config).to receive(:sleep).once { |*args| state[:time] = 0.6 } + it 'breaks' do + allow(config).to receive(:sleep).once { state[:time] = 0.6 } expect(config).to_not receive(:call) subject.loop_for(1) end end - context "when still paused after sleeping" do - context "when there were no events before" do + context 'when still paused after sleeping' do + context 'when there were no events before' do before do allow(config).to receive(:last_queue_event_time).and_return(nil) sequence[1.0] = :stopped end - it "sleeps for latency to possibly later optimize some events" do + it 'sleeps for latency to possibly later optimize some events' do # pretend we were woken up at 0.6 seconds since start - allow(config).to receive(:sleep).with(no_args) { |*args| state[:time] += 0.6 }.ordered + allow(config).to receive(:sleep). + with(no_args) { |*_args| state[:time] += 0.6 }.ordered # pretend we slept for latency (now: 1.6 seconds since start) - allow(config).to receive(:sleep).with(1.0) { |*args| state[:time] += 1.0}.ordered + allow(config).to receive(:sleep). + with(1.0) { |*_args| state[:time] += 1.0 }.ordered + subject.loop_for(1) end end - #context "when there were events within latency" do + # context "when there were events within latency" do # pending "todo" # fail "fix me" - #end + # end - context "when there were no events for ages" do + context 'when there were no events for ages' do before do allow(config).to receive(:last_queue_event_time).and_return(0.0) sequence[3.5] = :stopped # in the future to break from the loop end - it "still does not process events because it is paused" do + it 'still does not process events because it is paused' do # pretend we were woken up at 0.6 seconds since start - allow(config).to receive(:sleep).with(no_args) { |*args| state[:time] += 2.0 }.ordered + allow(config).to receive(:sleep). + with(no_args) { |*_args| state[:time] += 2.0 }.ordered - # second loop starts here (no sleep, coz recent events, but no processing coz paused + # second loop starts here (no sleep, coz recent events, but no + # processing coz paused # pretend we were woken up at 3.6 seconds since start - allow(config).to receive(:sleep).with(no_args) { |*args| state[:time] += 3.0 }.ordered + allow(config).to receive(:sleep). + with(no_args) { |*_args| state[:time] += 3.0 }.ordered subject.loop_for(1) end @@ -126,48 +132,52 @@ def status_for_time(time) end end - context "when initially processing" do + context 'when initially processing' do before do sequence[0.0] = :processing end - context "when event queue is empty" do + context 'when event queue is empty' do before do allow(event_queue).to receive(:empty?).and_return(true) end - context "when stopped after sleeping" do + context 'when stopped after sleeping' do before do sequence[0.2] = :stopped end - it "sleeps, waiting to be woken up" do - expect(config).to receive(:sleep).once { |*args| state[:time] = 0.6 } + it 'sleeps, waiting to be woken up' do + expect(config).to receive(:sleep). + once { |*_args| state[:time] = 0.6 } + subject.loop_for(1) end - it "breaks" do - allow(config).to receive(:sleep).once { |*args| state[:time] = 0.6 } + it 'breaks' do + allow(config).to receive(:sleep). + once { |*_args| state[:time] = 0.6 } + expect(config).to_not receive(:call) subject.loop_for(1) end end end - context "when event queue has events" do + context 'when event queue has events' do before do end - context "when there were events ages ago" do + context 'when there were events ages ago' do before do sequence[3.5] = :stopped # in the future to break from the loop end - it "processes events" do - allow(event_queue).to receive(:empty?).and_return(false, false, false, true) + it 'processes events' do + allow(event_queue).to receive(:empty?). + and_return(false, false, false, true) + allow(config).to receive(:last_queue_event_time).and_return(-3.0) - # pretend we were woken up at 0.6 seconds since start - #allow(config).to receive(:sleep).with(no_args) { |*args| state[:time] += 2.0 }.ordered # resets latency check expect(config).to receive(:reset_last_queue_event_time) @@ -176,10 +186,13 @@ def status_for_time(time) change = [:file, :modified, 'foo', 'bar'] resulting_changes = { modified: ['foo'], added: [], removed: [] } allow(event_queue).to receive(:pop).and_return(change).ordered - allow(config).to receive(:optimize_changes).with([change]).and_return(resulting_changes) + + allow(config).to receive(:optimize_changes).with([change]). + and_return(resulting_changes) + final_changes = [['foo'], [], []] allow(config).to receive(:call) do |*changes| - state[:time] = 4.0 #stopped + state[:time] = 4.0 # stopped expect(changes).to eq(final_changes) end @@ -187,11 +200,11 @@ def status_for_time(time) end end - # context "when stopped after sleeping" do - # it "breaks from the loop" do - # pending "todo" - # end - # end + # context "when stopped after sleeping" do + # it "breaks from the loop" do + # pending "todo" + # end + # end end end end diff --git a/spec/lib/listen/file_spec.rb b/spec/lib/listen/file_spec.rb index aa0a9e6d..d20c4c2f 100644 --- a/spec/lib/listen/file_spec.rb +++ b/spec/lib/listen/file_spec.rb @@ -70,7 +70,8 @@ it { should be :modified } it 'sets path in record with expected data' do - expect(record).to receive(:update_file).with('file.rb', expected_data) + expect(record).to receive(:update_file). + with('file.rb', expected_data) subject end end From 2853f178f8264c0dc0a428acd7f0324433b35690 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Fri, 12 Jun 2015 13:01:34 +0200 Subject: [PATCH 10/47] remove Celluloid --- Gemfile | 2 - lib/listen/adapter/base.rb | 5 +- lib/listen/adapter/linux.rb | 3 - lib/listen/fsm.rb | 131 ++++++++++++++++++++++++++ lib/listen/internals/thread_pool.rb | 1 - lib/listen/listener.rb | 102 +++++--------------- listen.gemspec | 4 - spec/lib/listen/adapter/linux_spec.rb | 8 +- spec/lib/listen/listener_spec.rb | 27 +----- spec/spec_helper.rb | 3 - spec/support/acceptance_helper.rb | 2 +- 11 files changed, 161 insertions(+), 127 deletions(-) create mode 100644 lib/listen/fsm.rb diff --git a/Gemfile b/Gemfile index 1ccb6ca6..5619019f 100644 --- a/Gemfile +++ b/Gemfile @@ -7,13 +7,11 @@ require 'rbconfig' case RbConfig::CONFIG['target_os'] when /mswin|mingw|cygwin/i gem 'wdm', '>= 0.1.0' - Kernel.warn 'NOTE: Celluloid may not work properly on your platform' when /bsd|dragonfly/i gem 'rb-kqueue', '>= 0.2' end group :test do - gem 'celluloid', github: 'celluloid/celluloid', branch: '0-16-stable' gem 'rake' gem 'rspec', '~> 3.2' gem 'rspec-retry' diff --git a/lib/listen/adapter/base.rb b/lib/listen/adapter/base.rb index 83771e79..247277c5 100644 --- a/lib/listen/adapter/base.rb +++ b/lib/listen/adapter/base.rb @@ -3,8 +3,6 @@ module Listen module Adapter class Base - include Celluloid - attr_reader :options # TODO: only used by tests @@ -78,8 +76,7 @@ def self.usable? private def _queue_change(type, dir, rel_path, options) - # TODO: temporary workaround to remove dependency on Change through - # Celluloid in tests + # TODO: temporary workaround to remove dependency on Change @mq.send(:_queue_raw_change, type, dir, rel_path, options) end diff --git a/lib/listen/adapter/linux.rb b/lib/listen/adapter/linux.rb index 72f5e005..17cc4399 100644 --- a/lib/listen/adapter/linux.rb +++ b/lib/listen/adapter/linux.rb @@ -30,9 +30,6 @@ def _configure(directory, &callback) @worker ||= INotify::Notifier.new @worker.watch(directory.to_s, *options.events, &callback) rescue Errno::ENOSPC - # workaround - Celluloid catches abort and prints nothing - STDERR.puts INOTIFY_LIMIT_MESSAGE - STDERR.flush abort(INOTIFY_LIMIT_MESSAGE) end diff --git a/lib/listen/fsm.rb b/lib/listen/fsm.rb new file mode 100644 index 00000000..88946f23 --- /dev/null +++ b/lib/listen/fsm.rb @@ -0,0 +1,131 @@ +# Code copied from https://github.com/celluloid/celluloid-fsm +module Listen + module FSM + DEFAULT_STATE = :default # Default state name unless one is explicitly set + + # Included hook to extend class methods + def self.included(klass) + klass.send :extend, ClassMethods + end + + module ClassMethods + # Obtain or set the default state + # Passing a state name sets the default state + def default_state(new_default = nil) + if new_default + @default_state = new_default.to_sym + else + defined?(@default_state) ? @default_state : DEFAULT_STATE + end + end + + # Obtain the valid states for this FSM + def states + @states ||= {} + end + + # Declare an FSM state and optionally provide a callback block to fire + # Options: + # * to: a state or array of states this state can transition to + def state(*args, &block) + if args.last.is_a? Hash + # Stringify keys :/ + options = args.pop.each_with_object({}) { |(k, v), h| h[k.to_s] = v } + else + options = {} + end + + args.each do |name| + name = name.to_sym + default_state name if options['default'] + states[name] = State.new(name, options['to'], &block) + end + end + end + + # Be kind and call super if you must redefine initialize + def initialize + @state = self.class.default_state + end + + # Obtain the current state of the FSM + attr_reader :state + + def transition(state_name) + new_state = validate_and_sanitize_new_state(state_name) + return unless new_state + transition_with_callbacks!(new_state) + end + + # Immediate state transition with no checks, or callbacks. "Dangerous!" + def transition!(state_name) + @state = state_name + end + + protected + + def validate_and_sanitize_new_state(state_name) + state_name = state_name.to_sym + + return if current_state_name == state_name + + if current_state && !current_state.valid_transition?(state_name) + valid = current_state.transitions.map(&:to_s).join(', ') + msg = "#{self.class} can't change state from '#{@state}'"\ + " to '#{state_name}', only to: #{valid}" + fail ArgumentError, msg + end + + new_state = states[state_name] + + unless new_state + return if state_name == default_state + fail ArgumentError, "invalid state for #{self.class}: #{state_name}" + end + + new_state + end + + def transition_with_callbacks!(state_name) + transition! state_name.name + state_name.call(self) + end + + def states + self.class.states + end + + def default_state + self.class.default_state + end + + def current_state + states[@state] + end + + def current_state_name + current_state && current_state.name || '' + end + + class State + attr_reader :name, :transitions + + def initialize(name, transitions = nil, &block) + @name, @block = name, block + @transitions = nil + @transitions = Array(transitions).map(&:to_sym) if transitions + end + + def call(obj) + obj.instance_eval(&@block) if @block + end + + def valid_transition?(new_state) + # All transitions are allowed unless expressly + return true unless @transitions + + @transitions.include? new_state.to_sym + end + end + end +end diff --git a/lib/listen/internals/thread_pool.rb b/lib/listen/internals/thread_pool.rb index 2d29da19..bb276592 100644 --- a/lib/listen/internals/thread_pool.rb +++ b/lib/listen/internals/thread_pool.rb @@ -1,7 +1,6 @@ module Listen # @private api module Internals - # Just a wrapper for tests to avoid interfereing with Celluloid's threads module ThreadPool def self.add(&block) Thread.new { block.call }.tap do |th| diff --git a/lib/listen/listener.rb b/lib/listen/listener.rb index ff82b8d9..52ea22cf 100644 --- a/lib/listen/listener.rb +++ b/lib/listen/listener.rb @@ -14,13 +14,13 @@ require 'listen/internals/logging' +require 'listen/fsm' require 'listen/event_processor' - module Listen class Listener - include Celluloid::FSM + include Listen::FSM attr_accessor :block @@ -28,8 +28,6 @@ class Listener # TODO: deprecate attr_reader :options, :directories - attr_reader :registry, :supervisor - # Initializes the directories listener. # @@ -44,20 +42,31 @@ class Listener def initialize(*args, &block) @options = _init_options(args.last.is_a?(Hash) ? args.pop : {}) - # Setup logging first - if Celluloid.logger - Celluloid.logger.level = _debug_level - _info "Celluloid loglevel set to: #{Celluloid.logger.level}" - _info "Listen version: #{Listen::VERSION}" - end - @silencer = Silencer.new @silencer_controller = Silencer::Controller.new(@silencer, @options) @directories = args.flatten.map { |path| Pathname.new(path).realpath } @event_queue = Queue.new @block = block - @registry = Celluloid::Registry.new + + change_config = Listen::Change::Config.new(self) + changes = @directories.map do |dir| + [ + dir.to_s, + Change.new(change_config, Record.new(dir)) + ] + end + @fs_changes = Hash[changes] + + adapter_options = { mq: self, directories: directories } + + # TODO: refactor + valid_adapter_options = _adapter_class.const_get(:DEFAULTS).keys + valid_adapter_options.each do |key| + adapter_options.merge!(key => options[key]) if options.key?(key) + end + + @adapter = Adapter.select(options).new(adapter_options) optimizer_config = QueueOptimizer::Config.new(@adapter.class, @silencer) @queue_optimizer = QueueOptimizer.new(optimizer_config) @@ -72,10 +81,6 @@ def initialize(*args, &block) state :stopped, to: [:processing] do _stop_wait_thread - if @supervisor - @supervisor.terminate - @supervisor = nil - end end state :processing, to: [:paused, :stopped] do @@ -84,7 +89,6 @@ def initialize(*args, &block) else self.last_queue_event_time = nil _start_wait_thread - _init_actors begin start = Time.now.to_f @@ -97,7 +101,7 @@ def initialize(*args, &block) raise end - _start_adapter + @adapter.start end end @@ -158,15 +162,6 @@ def only(regexps) @silencer_controller.replace_with_only(regexps) end - def async(type) - proxy = sync(type) - proxy ? proxy.async : nil - end - - def sync(type) - @registry[type] - end - def queue(type, change, dir, path, options = {}) fail "Invalid type: #{type.inspect}" unless [:dir, :file].include? type fail "Invalid change: #{change.inspect}" unless change.is_a?(Symbol) @@ -207,49 +202,6 @@ def _init_options(options = {}) }.merge(options) end - def _debug_level - debugging = ENV['LISTEN_GEM_DEBUGGING'] || options[:debug] - case debugging.to_s - when /2/ - Logger::DEBUG - when /true|yes|1/i - Logger::INFO - else - Logger::ERROR - end - end - - def _init_actors - adapter_options = { mq: self, directories: directories } - - @supervisor = Celluloid::SupervisionGroup.run!(registry) - - # TODO: broadcaster should be a separate plugin - if @tcp_mode == :broadcaster - require 'listen/tcp/broadcaster' - - # TODO: pass a TCP::Config class to make sure host and port are properly - # passed, even when nil - supervisor.add(TCP::Broadcaster, as: :broadcaster, args: [@host, @port]) - - # TODO: should be auto started, because if it crashes - # a new instance is spawned by supervisor, but it's 'start' isn't - # called - registry[:broadcaster].start - elsif @tcp_mode == :recipient - # TODO: adapter options should be configured in Listen.{on/to} - adapter_options.merge!(host: @host, port: @port) - end - - # TODO: refactor - valid_adapter_options = _adapter_class.const_get(:DEFAULTS).keys - valid_adapter_options.each do |key| - adapter_options.merge!(key => options[key]) if options.key?(key) - end - - supervisor.add(_adapter_class, as: :adapter, args: [adapter_options]) - end - def _wait_for_changes(config) latency = options[:wait_for_delay] EventProcessor.new(config).loop_for(latency) @@ -263,16 +215,6 @@ def _silenced?(path, type) @silencer.silenced?(path, type) end - def _start_adapter - # Don't run async, because configuration has to finish first - adapter = sync(:adapter) - adapter.start - end - - def _adapter_class - @adapter_class ||= Adapter.select(options) - end - attr_reader :adapter attr_reader :queue_optimizer attr_reader :event_queue diff --git a/listen.gemspec b/listen.gemspec index dfaa6a4b..1a0e9434 100644 --- a/listen.gemspec +++ b/listen.gemspec @@ -24,10 +24,6 @@ Gem::Specification.new do |s| s.required_ruby_version = '>= 1.9.3' - # NOTE: different minor versions are API-incompatible, - # so we stick to the latest stable+maintained version - s.add_dependency 'celluloid', '~> 0.16.0' - s.add_dependency 'rb-fsevent', '>= 0.9.3' s.add_dependency 'rb-inotify', '>= 0.9' diff --git a/spec/lib/listen/adapter/linux_spec.rb b/spec/lib/listen/adapter/linux_spec.rb index 3cbec225..7a316841 100644 --- a/spec/lib/listen/adapter/linux_spec.rb +++ b/spec/lib/listen/adapter/linux_spec.rb @@ -16,7 +16,6 @@ subject { described_class.new(mq: mq, directories: directories) } - # workaround: Celluloid ignores SystemExit exception messages describe 'inotify limit message' do let(:directories) { [Pathname.pwd] } @@ -31,12 +30,7 @@ it 'should be shown before calling abort' do expected_message = described_class.const_get('INOTIFY_LIMIT_MESSAGE') - expect(STDERR).to receive(:puts).with(expected_message) - - # Expect RuntimeError here, for the sake of unit testing (actual - # handling depends on Celluloid supervisor setup, which is beyond the - # scope of subject tests) - expect { subject.start }.to raise_error RuntimeError, expected_message + expect { subject.start }.to raise_error SystemExit, expected_message end end diff --git a/spec/lib/listen/listener_spec.rb b/spec/lib/listen/listener_spec.rb index 96875d34..7314a25b 100644 --- a/spec/lib/listen/listener_spec.rb +++ b/spec/lib/listen/listener_spec.rb @@ -110,26 +110,10 @@ describe '#start' do before do - allow(subject).to receive(:_start_adapter) + allow(adapter).to receive(:start) allow(silencer).to receive(:silenced?) { false } end - it 'supervises adapter' do - allow(Adapter).to receive(:select) { Adapter::Polling } - options = [mq: subject, directories: []] - expect(supervisor).to receive(:add). - with(Adapter::Polling, as: :adapter, args: options) - - subject.start - end - - it 'supervises record' do - expect(supervisor).to receive(:add). - with(Record, as: :record, args: subject) - - subject.start - end - it 'builds record' do expect(record).to receive(:build) subject.start @@ -141,7 +125,7 @@ end it 'starts adapter' do - expect(subject).to receive(:_start_adapter) + expect(adapter).to receive(:start) subject.start end @@ -227,12 +211,10 @@ describe '#stop' do before do - allow(Celluloid::SupervisionGroup).to receive(:run!) { supervisor } subject.start end - it 'terminates supervisor' do - expect(supervisor).to receive(:terminate) + it 'terminates' do subject.stop end end @@ -388,11 +370,12 @@ context 'when listener is stopped' do before do subject.stop + allow(silencer).to receive(:silenced?) { true } end it 'queuing does not crash when changes come in' do expect do - subject.send(:_queue_raw_change, :dir, dir, 'path', recursive: true) + subject.send(:_queue_raw_change, :dir, realdir1, 'path', recursive: true) end.to_not raise_error end end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index dd36a97e..bca36edb 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -36,16 +36,13 @@ def ci? config.default_retry_count = ci? ? 5 : 1 end -require 'celluloid/rspec' Thread.abort_on_exception = true Listen.logger.level = Logger::DEBUG RSpec.configuration.before(:each) do Listen::Internals::ThreadPool.stop - Celluloid.boot end RSpec.configuration.after(:each) do - Celluloid.shutdown Listen::Internals::ThreadPool.stop end diff --git a/spec/support/acceptance_helper.rb b/spec/support/acceptance_helper.rb index 9ea48eb4..4827def5 100644 --- a/spec/support/acceptance_helper.rb +++ b/spec/support/acceptance_helper.rb @@ -194,7 +194,7 @@ def listen(reset_queue = true) yield # Polling sleep (default: 1s) - adapter = @listener.sync(:adapter) + adapter = @listener.send(:adapter) sleep(1.0) if adapter.is_a?(Listen::Adapter::Polling) # Lag should include: From 83e24e201f4d504ccc626258c5e4643ed011e0d9 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Fri, 12 Jun 2015 14:19:55 +0200 Subject: [PATCH 11/47] rework Listener spec --- spec/lib/listen/listener_spec.rb | 123 ++++++++++++++++++++++++------- spec/lib/listen/record_spec.rb | 2 +- 2 files changed, 98 insertions(+), 27 deletions(-) diff --git a/spec/lib/listen/listener_spec.rb b/spec/lib/listen/listener_spec.rb index 7314a25b..040cbdeb 100644 --- a/spec/lib/listen/listener_spec.rb +++ b/spec/lib/listen/listener_spec.rb @@ -1,25 +1,75 @@ include Listen RSpec.describe Listener do - subject { described_class.new(options) } + + let(:realdir1) { instance_double(Pathname, '/foo/dir1', children: []) } + let(:realdir2) { instance_double(Pathname, '/foo/dir2', children: []) } + + let(:queue) { instance_double(Queue) } + + let(:dir1) { instance_double(Pathname, 'dir1', realpath: realdir1) } + let(:dir2) { instance_double(Pathname, 'dir2', realpath: realdir2) } + + let(:dirs) { ['dir1'] } + + subject { described_class.new(*(dirs + [options]).compact) } + let(:options) { {} } - let(:registry) { instance_double(Celluloid::Registry) } - let(:supervisor) do - instance_double(Celluloid::SupervisionGroup, add: true, pool: true) + let(:record) { instance_double(Record, build: true, root: 'dir2') } + let(:silencer) { instance_double(Silencer, configure: nil) } + + let(:adapter_namespace) do + class_double('Listen::Adapter'). + as_stubbed_const(transfer_nested_constants: true) end - let(:record) { instance_double(Record, terminate: true, build: true) } - let(:silencer) { instance_double(Silencer, configure: nil) } - let(:adapter) { instance_double(Adapter::Base, start: nil) } + let(:adapter_class) { class_double('Listen::Adapter::Base') } + let(:adapter) { instance_double('Listen::Adapter::Base', start: nil) } + + let(:optimizer_config) { instance_double(QueueOptimizer::Config) } + let(:optimizer) { instance_double(QueueOptimizer) } + + let(:processor_config) { instance_double(EventProcessor::Config) } + let(:processor) { instance_double(EventProcessor) } + + let(:default_latency) { 0.1 } before do - allow(Listen::Silencer).to receive(:new) { silencer } + allow(Silencer).to receive(:new) { silencer } + + # TODO: use a configuration object to clean this up + allow(adapter_namespace).to receive(:select). + with(anything).and_return(adapter_class) + + allow(adapter_class).to receive(:new).with(anything).and_return(adapter) + allow(adapter_class).to receive(:local_fs?).and_return(true) + allow(adapter).to receive(:class).and_return(adapter_class) + + allow(QueueOptimizer::Config).to receive(:new). + with(adapter_class, silencer).and_return(optimizer_config) + + allow(QueueOptimizer).to receive(:new).with(optimizer_config). + and_return(optimizer) - allow(Celluloid::Registry).to receive(:new) { registry } - allow(Celluloid::SupervisionGroup).to receive(:run!) { supervisor } - allow(registry).to receive(:[]).with(:adapter) { adapter } - allow(registry).to receive(:[]).with(:record) { record } + allow(EventProcessor::Config).to receive(:new). + with(anything, queue, optimizer).and_return(processor_config) + + allow(EventProcessor).to receive(:new).with(processor_config). + and_return(processor) + + allow(processor).to receive(:loop_for).with(default_latency) + + allow(Record).to receive(:new).and_return(record) + + allow(Pathname).to receive(:new).with('dir1').and_return(dir1) + allow(Pathname).to receive(:new).with('dir2').and_return(dir2) + + allow(Queue).to receive(:new).and_return(queue) + allow(queue).to receive(:<<) + allow(queue).to receive(:empty?).and_return(true) + + allow(Internals::ThreadPool).to receive(:add) end describe 'initialize' do @@ -27,16 +77,15 @@ context 'with a block' do describe 'block' do - subject { described_class.new('lib', &(proc {})) } + subject { described_class.new('dir1', &(proc {})) } specify { expect(subject.block).to_not be_nil } end end context 'with directories' do describe 'directories' do - subject { described_class.new('lib', 'spec') } - expected = %w(lib spec).map { |dir| Pathname.pwd + dir } - specify { expect(subject.directories).to eq expected } + subject { described_class.new('dir1', 'dir2') } + specify { expect(subject.directories).to eq([realdir1, realdir2]) } end end end @@ -89,7 +138,7 @@ context 'custom options' do subject do described_class.new( - 'lib', + 'dir1', latency: 1.234, wait_for_delay: 0.85, relative: true) @@ -154,7 +203,7 @@ subject.start subject.queue(:file, :modified, event_dir, 'foo') - subject.block.call(['foo'], [], [] ) + subject.block.call(['foo'], [], []) sleep 0.25 end end @@ -179,7 +228,7 @@ subject.start subject.queue(:file, :modified, event_dir, 'foo') - subject.block.call(['../foo'], [], [] ) + subject.block.call(['../foo'], [], []) end end @@ -187,8 +236,15 @@ let(:options) { { relative: true } } it 'registers full path' do - event_dir = instance_double(Pathname) - foo_rel_path = instance_double(Pathname, to_s: 'd:/foo', exist?: true) + event_dir = instance_double(Pathname, 'event_dir', realpath: 'd:/foo') + + foo_rel_path = instance_double( + Pathname, + 'rel_path', + to_s: 'd:/foo', + exist?: true, + children: [] + ) allow(event_dir).to receive(:relative_path_from). with(Pathname.pwd). @@ -202,7 +258,7 @@ subject.start subject.queue(:file, :modified, event_dir, 'foo') - subject.block.call(['d:/foo'], [], [] ) + subject.block.call(['d:/foo'], [], []) end end @@ -281,8 +337,12 @@ it 'adds up to existing ignore options' do expect(silencer).to receive(:configure).once.with(ignore: [/bar/]) + subject - expect(silencer).to receive(:configure).once.with(ignore: [/bar/, /foo/]) + + expect(silencer).to receive(:configure).once. + with(ignore: [/bar/, /foo/]) + subject.ignore(/foo/) end end @@ -292,8 +352,12 @@ it 'adds up to existing ignore options' do expect(silencer).to receive(:configure).once.with(ignore: [/bar/]) + subject - expect(silencer).to receive(:configure).once.with(ignore: [/bar/, /foo/]) + + expect(silencer).to receive(:configure).once. + with(ignore: [/bar/, /foo/]) + subject.ignore(/foo/) end end @@ -363,7 +427,7 @@ subject.start subject.queue(:file, :modified, dir, 'bar.txt', {}) subject.queue(:file, :added, dir, 'baz.txt', {}) - subject.block.call(['foo/bar.txt'], ['foo/baz.txt'], [] ) + subject.block.call(['foo/bar.txt'], ['foo/baz.txt'], []) end end @@ -375,7 +439,14 @@ it 'queuing does not crash when changes come in' do expect do - subject.send(:_queue_raw_change, :dir, realdir1, 'path', recursive: true) + # TODO: write directly to queue + subject.send( + :_queue_raw_change, + :dir, + realdir1, + 'path', + recursive: true) + end.to_not raise_error end end diff --git a/spec/lib/listen/record_spec.rb b/spec/lib/listen/record_spec.rb index 08e0939a..b7a7a7ac 100644 --- a/spec/lib/listen/record_spec.rb +++ b/spec/lib/listen/record_spec.rb @@ -139,7 +139,7 @@ def record_tree(record) it 'unsets path' do record.unset_path('path/file.rb') - expect(record_tree(record)).to eq({ 'path' => {} }) + expect(record_tree(record)).to eq('path' => {}) end end From f765f7fb56d5577920bb794ae03d5d33d8306f57 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:57:00 +0200 Subject: [PATCH 12/47] add fake_path spec helper --- spec/spec_helper.rb | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index bca36edb..428ee32c 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -31,8 +31,15 @@ def ci? config.disable_monkey_patching! end +module SpecHelpers + def fake_path(str, options = {}) + instance_double(Pathname, str, { to_s: str }.merge(options)) + end +end + require 'rspec/retry' RSpec.configure do |config| + config.include SpecHelpers config.default_retry_count = ci? ? 5 : 1 end From e91c7642de01b75be330ffc2b1dd30ebbe471283 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:27:24 +0200 Subject: [PATCH 13/47] add new refactored Event::* classes --- lib/listen/event/config.rb | 59 +++++ lib/listen/event/loop.rb | 113 ++++++++++ lib/listen/event/processor.rb | 122 +++++++++++ lib/listen/event/queue.rb | 54 +++++ lib/listen/event_processor.rb | 111 ---------- spec/lib/listen/event/config_spec.rb | 31 +++ spec/lib/listen/event/loop_spec.rb | 204 ++++++++++++++++++ .../processor_spec.rb} | 48 +++-- spec/lib/listen/event/queue_spec.rb | 138 ++++++++++++ 9 files changed, 747 insertions(+), 133 deletions(-) create mode 100644 lib/listen/event/config.rb create mode 100644 lib/listen/event/loop.rb create mode 100644 lib/listen/event/processor.rb create mode 100644 lib/listen/event/queue.rb delete mode 100644 lib/listen/event_processor.rb create mode 100644 spec/lib/listen/event/config_spec.rb create mode 100644 spec/lib/listen/event/loop_spec.rb rename spec/lib/listen/{event_processor_spec.rb => event/processor_spec.rb} (86%) create mode 100644 spec/lib/listen/event/queue_spec.rb diff --git a/lib/listen/event/config.rb b/lib/listen/event/config.rb new file mode 100644 index 00000000..ee13bd65 --- /dev/null +++ b/lib/listen/event/config.rb @@ -0,0 +1,59 @@ +module Listen + module Event + class Config + def initialize( + listener, + event_queue, + queue_optimizer, + wait_for_delay, + &block) + + @listener = listener + @event_queue = event_queue + @queue_optimizer = queue_optimizer + @min_delay_between_events = wait_for_delay + @block = block + end + + def sleep(*args) + Kernel.sleep(*args) + end + + def call(*args) + @block.call(*args) if @block + end + + def timestamp + Time.now.to_f + end + + def event_queue + @event_queue + end + + def callable? + @block + end + + def optimize_changes(changes) + @queue_optimizer.smoosh_changes(changes) + end + + def min_delay_between_events + @min_delay_between_events + end + + def stopped? + listener.state == :stopped + end + + def paused? + listener.state == :paused + end + + private + + attr_reader :listener + end + end +end diff --git a/lib/listen/event/loop.rb b/lib/listen/event/loop.rb new file mode 100644 index 00000000..8c2ae788 --- /dev/null +++ b/lib/listen/event/loop.rb @@ -0,0 +1,113 @@ +require 'listen/event/processor' + +module Listen + module Event + class Loop + class Error < RuntimeError + class NotStarted < Error + end + end + + def initialize(config) + @config = config + @wait_thread = nil + @state = :paused + @reasons = Thread::Queue.new + end + + def wakeup_on_event + return if stopped? + return unless processing? + return unless wait_thread.alive? + _wakeup(:event) + end + + def paused? + wait_thread && state == :paused + end + + def processing? + return false if stopped? + return false if paused? + state == :processing + end + + def setup + # TODO: use a Fiber instead? + q = Thread::Queue.new + @wait_thread = Internals::ThreadPool.add do + _wait_for_changes(q, config) + end + + Listen::Logger.debug('Waiting for processing to start...') + Timeout.timeout(5) { q.pop } + end + + def resume + fail Error::NotStarted if stopped? + return unless wait_thread + _wakeup(:resume) + end + + def pause + fail NotImplementedError + end + + def teardown + return unless wait_thread + if wait_thread.alive? + _wakeup(:teardown) + wait_thread.join + end + @wait_thread = nil + end + + def stopped? + !wait_thread + end + + private + + attr_reader :config + attr_reader :wait_thread + + attr_accessor :state + + def _wait_for_changes(ready_queue, config) + processor = Event::Processor.new(config, @reasons) + + _wait_until_resumed(ready_queue) + processor.loop_for(config.min_delay_between_events) + rescue StandardError => ex + _nice_error(ex) + end + + def _sleep(*args) + Kernel.sleep(*args) + end + + def _wait_until_resumed(ready_queue) + self.state = :paused + ready_queue << :ready + sleep + self.state = :processing + end + + def _nice_error(ex) + indent = "\n -- " + msg = format( + 'exception while processing events: %s Backtrace:%s%s', + ex, + indent, + ex.backtrace * indent + ) + Listen::Logger.error(msg) + end + + def _wakeup(reason) + @reasons << reason + wait_thread.wakeup + end + end + end +end diff --git a/lib/listen/event/processor.rb b/lib/listen/event/processor.rb new file mode 100644 index 00000000..ab492e73 --- /dev/null +++ b/lib/listen/event/processor.rb @@ -0,0 +1,122 @@ +module Listen + module Event + class Processor + def initialize(config, reasons) + @config = config + @reasons = reasons + _reset_no_unprocessed_events + end + + # TODO: implement this properly instead of checking the state at arbitrary + # points in time + def loop_for(latency) + @latency = latency + + loop do + _wait_until_events + _wait_until_events_calm_down + _wait_until_no_longer_paused + _process_changes + end + rescue Stopped + Listen::Logger.debug('Processing stopped') + end + + private + + class Stopped < RuntimeError + end + + def _wait_until_events_calm_down + loop do + now = _timestamp + + # Assure there's at least latency between callbacks to allow + # for accumulating changes + diff = _deadline - now + break if diff <= 0 + + # give events a bit of time to accumulate so they can be + # compressed/optimized + _sleep(:waiting_until_latency, diff) + end + end + + def _wait_until_no_longer_paused + # TODO: may not be a good idea? + _sleep(:waiting_for_unpause) while config.paused? + end + + def _check_stopped + return unless config.stopped? + + _flush_wakeup_reasons + raise Stopped + end + + def _sleep(_local_reason, *args) + _check_stopped + sleep_duration = config.sleep(*args) + _check_stopped + + _flush_wakeup_reasons do |reason| + next unless reason == :event + _remember_time_of_first_unprocessed_event unless config.paused? + end + + sleep_duration + end + + def _remember_time_of_first_unprocessed_event + @first_unprocessed_event_time ||= _timestamp + end + + def _reset_no_unprocessed_events + @first_unprocessed_event_time = nil + end + + def _deadline + @first_unprocessed_event_time + @latency + end + + def _wait_until_events + # TODO: long sleep may not be a good idea? + _sleep(:waiting_for_events) while config.event_queue.empty? + @first_unprocessed_event_time ||= _timestamp + end + + def _flush_wakeup_reasons + reasons = @reasons + until reasons.empty? + reason = reasons.pop + yield reason if block_given? + end + end + + def _timestamp + config.timestamp + end + + # for easier testing without sleep loop + def _process_changes + _reset_no_unprocessed_events + + changes = [] + changes << config.event_queue.pop until config.event_queue.empty? + + callable = config.callable? + return unless callable + + hash = config.optimize_changes(changes) + result = [hash[:modified], hash[:added], hash[:removed]] + return if result.all?(&:empty?) + + block_start = _timestamp + config.call(*result) + Listen::Logger.debug "Callback took #{_timestamp - block_start} sec" + end + + attr_reader :config + end + end +end diff --git a/lib/listen/event/queue.rb b/lib/listen/event/queue.rb new file mode 100644 index 00000000..642ef535 --- /dev/null +++ b/lib/listen/event/queue.rb @@ -0,0 +1,54 @@ +module Listen + module Event + class Queue + class Config + def initialize(relative) + @relative = relative + end + + def relative? + @relative + end + end + + def initialize(config, &block) + @event_queue = Thread::Queue.new + @block = block + @config = config + end + + def <<(args) + type, change, dir, path, options = *args + fail "Invalid type: #{type.inspect}" unless [:dir, :file].include? type + fail "Invalid change: #{change.inspect}" unless change.is_a?(Symbol) + fail "Invalid path: #{path.inspect}" unless path.is_a?(String) + + dir = _safe_relative_from_cwd(dir) + event_queue.public_send(:<<, [type, change, dir, path, options]) + + block.call(args) if block + end + + def empty? + event_queue.empty? + end + + def pop + event_queue.pop + end + + private + + attr_reader :event_queue + attr_reader :block + attr_reader :config + + def _safe_relative_from_cwd(dir) + return dir unless config.relative? + dir.relative_path_from(Pathname.pwd) + rescue ArgumentError + dir + end + end + end +end diff --git a/lib/listen/event_processor.rb b/lib/listen/event_processor.rb deleted file mode 100644 index 9f00f8d7..00000000 --- a/lib/listen/event_processor.rb +++ /dev/null @@ -1,111 +0,0 @@ -module Listen - class EventProcessor - class Config - def initialize(listener, event_queue, queue_optimizer) - @listener = listener - @event_queue = event_queue - @queue_optimizer = queue_optimizer - end - - def stopped? - listener.state == :stopped - end - - def paused? - listener.state == :paused - end - - def sleep(*args) - Kernel.sleep(*args) - end - - def call(*args) - listener.block.call(*args) - end - - def last_queue_event_time - # TODO: fix - listener.send(:last_queue_event_time) - end - - def timestamp - Time.now.to_f - end - - def event_queue - @event_queue - end - - def reset_last_queue_event_time - listener.send(:last_queue_event_time=, nil) - end - - def callable? - listener.block - end - - def optimize_changes(changes) - @queue_optimizer.smoosh_changes(changes) - end - - private - - attr_reader :listener - end - - def initialize(config) - @config = config - end - - # TODO: implement this properly instead of checking the state at arbitrary - # points in time - def loop_for(latency) - loop do - break if config.stopped? - - if config.paused? || config.event_queue.empty? - config.sleep - break if config.stopped? - end - - # Assure there's at least latency between callbacks to allow - # for accumulating changes - now = config.timestamp - diff = latency + (config.last_queue_event_time || now) - now - if diff > 0 - # give events a bit of time to accumulate so they can be - # compressed/optimized - config.sleep(diff) - next - end - - _process_changes unless config.paused? - end - end - - private - - # for easier testing without sleep loop - def _process_changes - return if config.event_queue.empty? # e.g. ignored changes - - config.reset_last_queue_event_time - - changes = [] - changes << config.event_queue.pop until config.event_queue.empty? - - callable = config.callable? - return unless callable - - hash = config.optimize_changes(changes) - result = [hash[:modified], hash[:added], hash[:removed]] - return if result.all?(&:empty?) - - block_start = config.timestamp - config.call(*result) - Listen.logger.debug "Callback took #{Time.now.to_f - block_start} seconds" - end - - attr_reader :config - end -end diff --git a/spec/lib/listen/event/config_spec.rb b/spec/lib/listen/event/config_spec.rb new file mode 100644 index 00000000..19827cff --- /dev/null +++ b/spec/lib/listen/event/config_spec.rb @@ -0,0 +1,31 @@ +require 'listen/event/config' + +RSpec.describe Listen::Event::Config do + let(:listener) { instance_double(Listen::Listener) } + let(:event_queue) { instance_double(Listen::Event::Queue) } + let(:queue_optimizer) { instance_double(Listen::QueueOptimizer) } + let(:wait_for_delay) { 1.234 } + + context 'with a given block' do + let(:myblock) { instance_double(Proc) } + + subject do + described_class.new( + listener, + event_queue, + queue_optimizer, + wait_for_delay) do |*args| + myblock.call(*args) + end + end + + it 'calls the block' do + expect(myblock).to receive(:call).with(:foo, :bar) + subject.call(:foo, :bar) + end + + it 'is callable' do + expect(subject).to be_callable + end + end +end diff --git a/spec/lib/listen/event/loop_spec.rb b/spec/lib/listen/event/loop_spec.rb new file mode 100644 index 00000000..e0ce1970 --- /dev/null +++ b/spec/lib/listen/event/loop_spec.rb @@ -0,0 +1,204 @@ +require 'listen/event/config' +require 'listen/event/loop' +require 'listen/internals/thread_pool' + +RSpec.describe Listen::Event::Loop do + let(:config) { instance_double(Listen::Event::Config, 'config') } + let(:processor) { instance_double(Listen::Event::Processor, 'processor') } + let(:thread) { instance_double(Thread) } + + let(:reasons) { instance_double(Thread::Queue, 'reasons') } + let(:ready) { instance_double(Thread::Queue, 'ready') } + + let(:blocks) do + { + thread_block: proc { fail 'thread block stub called' }, + timer_block: proc { fail 'thread block stub called' }, + } + end + + subject { described_class.new(config) } + + # TODO: this is hideous + before do + allow(Thread::Queue).to receive(:new).and_return(reasons, ready) + allow(Listen::Event::Processor).to receive(:new).with(config, reasons). + and_return(processor) + + allow(Listen::Internals::ThreadPool).to receive(:add) do |*args, &block| + fail 'Unstubbed call:'\ + " ThreadPool.add(#{args.map(&:inspect) * ','},&#{block.inspect})" + end + + allow(config).to receive(:min_delay_between_events).and_return(1.234) + + allow(Listen::Internals::ThreadPool).to receive(:add) do |*_, &block| + blocks[:thread_block] = block + thread + end + + allow(Timeout).to receive(:timeout) do |*_args, &block| + blocks[:timer_block] = block + end + + allow(Kernel).to receive(:sleep) do |*args| + fail "stub called: sleep(#{args.map(&:inspect) * ','})" + end + + allow(subject).to receive(:_nice_error) do |ex| + indent = "\n -- " + backtrace = ex.backtrace.reject { |line| line =~ /\/gems\// } + fail "error called: #{ex}: #{indent}#{backtrace * indent}" + end + end + + describe '#setup' do + before do + allow(thread).to receive(:wakeup) + allow(thread).to receive(:alive?).and_return(true) + allow(config).to receive(:min_delay_between_events).and_return(1.234) + allow(ready).to receive(:<<).with(:ready) + end + + it 'sets up the thread in a resumable state' do + subject.setup + + expect(subject).to receive(:sleep).with(no_args).ordered + allow(processor).to receive(:loop_for).with(1.234).ordered + + blocks[:thread_block].call + end + end + + context 'when stopped' do + context 'when resume is called' do + it 'fails' do + expect { subject.resume }. + to raise_error(Listen::Event::Loop::Error::NotStarted) + end + end + + context 'when wakeup_on_event is called' do + it 'does nothing' do + subject.wakeup_on_event + end + end + end + + context 'when resumed' do + before do + subject.setup + + allow(thread).to receive(:wakeup) do + allow(subject).to receive(:sleep).with(no_args).ordered + allow(processor).to receive(:loop_for).with(1.234).ordered + allow(ready).to receive(:<<).with(:ready) + blocks[:thread_block].call + end + + allow(reasons).to receive(:<<).with(:resume) + subject.resume + end + + it 'is not paused' do + expect(subject).to_not be_paused + end + + context 'when resume is called again' do + it 'does nothing' do + subject.resume + end + end + + context 'when wakeup_on_event is called' do + let(:epoch) { 1234 } + + context 'when thread is alive' do + before do + allow(reasons).to receive(:<<) + allow(thread).to receive(:alive?).and_return(true) + end + + it 'wakes up the thread' do + expect(thread).to receive(:wakeup) + subject.wakeup_on_event + end + + it 'sets the reason for waking up' do + expect(reasons).to receive(:<<).with(:event) + subject.wakeup_on_event + end + end + + context 'when thread is dead' do + before do + allow(thread).to receive(:alive?).and_return(false) + end + + it 'does not wake up the thread' do + expect(thread).to_not receive(:wakeup) + subject.wakeup_on_event + end + end + end + end + + context 'when set up / paused' do + before do + allow(thread).to receive(:alive?).and_return(true) + allow(config).to receive(:min_delay_between_events).and_return(1.234) + + allow(thread).to receive(:wakeup) + + subject.setup + + allow(subject).to receive(:sleep).with(no_args).ordered do + allow(processor).to receive(:loop_for).with(1.234) + blocks[:timer_block].call + end + + allow(ready).to receive(:<<).with(:ready) + allow(ready).to receive(:pop) + + blocks[:thread_block].call + end + + describe '#resume' do + before do + allow(reasons).to receive(:<<) + allow(thread).to receive(:wakeup) + end + + it 'resumes the thread' do + expect(thread).to receive(:wakeup) + subject.resume + end + + it 'sets the reason for waking up' do + expect(reasons).to receive(:<<).with(:resume) + subject.resume + end + end + + describe '#teardown' do + before do + allow(reasons).to receive(:<<) + allow(thread).to receive(:join) + end + + it 'frees the thread' do + subject.teardown + end + + it 'waits for the thread to finish' do + expect(thread).to receive(:join) + subject.teardown + end + + it 'sets the reason for waking up' do + expect(reasons).to receive(:<<).with(:teardown) + subject.teardown + end + end + end +end diff --git a/spec/lib/listen/event_processor_spec.rb b/spec/lib/listen/event/processor_spec.rb similarity index 86% rename from spec/lib/listen/event_processor_spec.rb rename to spec/lib/listen/event/processor_spec.rb index cc6858bf..b2e61113 100644 --- a/spec/lib/listen/event_processor_spec.rb +++ b/spec/lib/listen/event/processor_spec.rb @@ -1,18 +1,20 @@ -RSpec.describe Listen::EventProcessor do - let(:event_queue) { instance_double(Queue) } - let(:config) { instance_double(described_class::Config) } +require 'listen/event/processor' +require 'listen/event/config' - subject { described_class.new(config) } +RSpec.describe Listen::Event::Processor do + let(:event_queue) { instance_double(Thread::Queue, 'event_queue') } + let(:config) { instance_double(Listen::Event::Config) } + let(:reasons) { instance_double(Thread::Queue, 'reasons') } + subject { described_class.new(config, reasons) } + + # This is to simulate events over various points in time let(:sequence) do - { - } + {} end let(:state) do - { - time: 0 - } + { time: 0 } end def status_for_time(time) @@ -39,12 +41,20 @@ def status_for_time(time) end describe '#loop_for' do + before do + allow(reasons).to receive(:empty?).and_return(true) + end + context 'when stopped' do before do sequence[0.0] = :stopped end context 'with pending changes' do + before do + allow(event_queue).to receive(:empty?).and_return(false) + end + it 'does not change the event queue' do subject.loop_for(1) end @@ -60,6 +70,10 @@ def status_for_time(time) end context 'when not stopped' do + before do + allow(event_queue).to receive(:empty?).and_return(true) + end + context 'when initially paused' do before do sequence[0.0] = :paused @@ -85,7 +99,6 @@ def status_for_time(time) context 'when still paused after sleeping' do context 'when there were no events before' do before do - allow(config).to receive(:last_queue_event_time).and_return(nil) sequence[1.0] = :stopped end @@ -102,15 +115,8 @@ def status_for_time(time) end end - # context "when there were events within latency" do - # pending "todo" - # fail "fix me" - # end - context 'when there were no events for ages' do before do - allow(config).to receive(:last_queue_event_time).and_return(0.0) - sequence[3.5] = :stopped # in the future to break from the loop end @@ -175,17 +181,14 @@ def status_for_time(time) it 'processes events' do allow(event_queue).to receive(:empty?). - and_return(false, false, false, true) - - allow(config).to receive(:last_queue_event_time).and_return(-3.0) + and_return(false, false, true) # resets latency check - expect(config).to receive(:reset_last_queue_event_time) expect(config).to receive(:callable?).and_return(true) change = [:file, :modified, 'foo', 'bar'] resulting_changes = { modified: ['foo'], added: [], removed: [] } - allow(event_queue).to receive(:pop).and_return(change).ordered + allow(event_queue).to receive(:pop).and_return(change) allow(config).to receive(:optimize_changes).with([change]). and_return(resulting_changes) @@ -196,6 +199,7 @@ def status_for_time(time) expect(changes).to eq(final_changes) end + subject.instance_variable_set(:@first_unprocessed_event_time, -3) subject.loop_for(1) end end diff --git a/spec/lib/listen/event/queue_spec.rb b/spec/lib/listen/event/queue_spec.rb new file mode 100644 index 00000000..7bb3b24e --- /dev/null +++ b/spec/lib/listen/event/queue_spec.rb @@ -0,0 +1,138 @@ +require 'listen/event/queue' + +# TODO: not part of listener really +RSpec.describe Listen::Event::Queue do + let(:queue) { instance_double(Thread::Queue, 'my queue') } + + let(:config) { instance_double(Listen::Event::Queue::Config) } + + let(:relative) { false } + + let(:block) { proc {} } + + subject { described_class.new(config, &block) } + + before do + allow(config).to receive(:relative?).and_return(relative) + allow(Thread::Queue).to receive(:new).and_return(queue) + end + + describe '#empty?' do + before do + allow(queue).to receive(:empty?).and_return(empty) + end + + context 'when empty' do + let(:empty) { true } + it { is_expected.to be_empty } + end + + context 'when not empty' do + let(:empty) { false } + let(:watched_dir) { fake_path('watched_dir') } + before do + allow(queue).to receive(:empty?).and_return(false) + end + it { is_expected.to_not be_empty } + end + end + + describe '#pop' do + before do + allow(queue).to receive(:pop).and_return('foo') + end + + context 'when empty' do + let(:value) { 'foo' } + it 'forward the call to the queue' do + expect(subject.pop).to eq('foo') + end + end + end + + describe '#<<' do + let(:watched_dir) { fake_path('watched_dir') } + before do + allow(queue).to receive(:<<) + end + + context 'when a block is given' do + let(:calls) { [] } + let(:block) { proc { calls << 'called!' } } + + it 'calls the provided block' do + subject.<<([:file, :modified, watched_dir, 'foo', {}]) + expect(calls).to eq(['called!']) + end + end + + context 'when no block is given' do + let(:calls) { [] } + let(:block) { nil } + + it 'calls the provided block' do + subject.<<([:file, :modified, watched_dir, 'foo', {}]) + expect(calls).to eq([]) + end + end + + context 'when relative option is true' do + let(:relative) { true } + + context 'when watched dir is the current dir' do + let(:options) { { relative: true, directories: Pathname.pwd } } + + let(:dir_rel_path) { fake_path('.') } + let(:foo_rel_path) { fake_path('foo', exist?: true) } + + it 'registers relative paths' do + allow(dir_rel_path).to receive(:+).with('foo') { foo_rel_path } + + allow(watched_dir).to receive(:relative_path_from). + with(Pathname.pwd). + and_return(dir_rel_path) + + expect(queue).to receive(:<<). + with([:file, :modified, dir_rel_path, 'foo', {}]) + + subject.<<([:file, :modified, watched_dir, 'foo', {}]) + end + end + + context 'when watched dir is not the current dir' do + let(:options) { { relative: true } } + let(:dir_rel_path) { fake_path('..') } + let(:foo_rel_path) { fake_path('../foo', exist?: true) } + + it 'registers relative path' do + allow(watched_dir).to receive(:relative_path_from). + with(Pathname.pwd). + and_return(dir_rel_path) + + expect(queue).to receive(:<<). + with([:file, :modified, dir_rel_path, 'foo', {}]) + + subject.<<([:file, :modified, watched_dir, 'foo', {}]) + end + end + + context 'when watched dir is on another drive' do + let(:watched_dir) { fake_path('watched_dir', realpath: 'd:/foo') } + let(:foo_rel_path) { fake_path('d:/foo', exist?: true) } + + it 'registers full path' do + allow(watched_dir).to receive(:relative_path_from). + with(Pathname.pwd). + and_raise(ArgumentError) + + allow(watched_dir).to receive(:+).with('foo') { foo_rel_path } + + expect(queue).to receive(:<<). + with([:file, :modified, watched_dir, 'foo', {}]) + + subject.<<([:file, :modified, watched_dir, 'foo', {}]) + end + end + end + end +end From 40536d7e5fab993b1b5e4b20a866177233b30ef8 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:28:04 +0200 Subject: [PATCH 14/47] add Backend class for handling adapters --- lib/listen/backend.rb | 41 +++++++++++++++++ spec/lib/listen/backend_spec.rb | 80 +++++++++++++++++++++++++++++++++ 2 files changed, 121 insertions(+) create mode 100644 lib/listen/backend.rb create mode 100644 spec/lib/listen/backend_spec.rb diff --git a/lib/listen/backend.rb b/lib/listen/backend.rb new file mode 100644 index 00000000..a3e809b6 --- /dev/null +++ b/lib/listen/backend.rb @@ -0,0 +1,41 @@ +require 'listen/adapter' +require 'listen/adapter/base' +require 'listen/adapter/config' + +# This class just aggregates configuration object to avoid Listener specs +# from exploding with huge test setup blocks +module Listen + class Backend + def initialize(directories, queue, silencer, config) + adapter_select_opts = config.adapter_select_options + + adapter_class = Adapter.select(adapter_select_opts) + + # Use default from adapter if possible + @min_delay_between_events = config.min_delay_between_events + @min_delay_between_events ||= adapter_class::DEFAULTS[:wait_for_delay] + @min_delay_between_events ||= 0.1 + + adapter_opts = config.adapter_instance_options(adapter_class) + + aconfig = Adapter::Config.new(directories, queue, silencer, adapter_opts) + @adapter = adapter_class.new(aconfig) + end + + def start + adapter.start + end + + def stop + # TODO: does nothing + end + + def min_delay_between_events + @min_delay_between_events + end + + private + + attr_reader :adapter + end +end diff --git a/spec/lib/listen/backend_spec.rb b/spec/lib/listen/backend_spec.rb new file mode 100644 index 00000000..a41bfb66 --- /dev/null +++ b/spec/lib/listen/backend_spec.rb @@ -0,0 +1,80 @@ +require 'listen/backend' + +RSpec.describe Listen::Backend do + let(:dir1) { instance_double(Pathname, 'dir1', to_s: '/foo/dir1') } + + let(:silencer) { instance_double(Listen::Silencer) } + let(:queue) { instance_double(Queue) } + + let(:select_options) do + { force_polling: false, polling_fallback_message: 'foo' } + end + + let(:adapter_options) { { latency: 1234 } } + let(:options) { select_options.merge(adapter_options) } + + let(:adapter_config_class) { class_double('Listen::Adapter::Config') } + let(:adapter_config) { instance_double('Listen::Adapter::Config') } + + let(:config) { instance_double(Listen::Listener::Config) } + + subject { described_class.new([dir1], queue, silencer, config) } + + # Use Polling since it has a valid :latency option + let(:adapter_defaults) { { latency: 5.4321 } } + let(:adapter_class) { Listen::Adapter::Polling } + let(:adapter) { instance_double('Listen::Adapter::Polling') } + + let(:config_min_delay_between_events) { 0.1234 } + + before do + stub_const('Listen::Adapter::Config', adapter_config_class) + + allow(adapter_config_class).to receive(:new). + with([dir1], queue, silencer, adapter_options). + and_return(adapter_config) + + allow(Listen::Adapter).to receive(:select). + with(select_options).and_return(adapter_class) + + allow(adapter_class).to receive(:new). + with(adapter_config).and_return(adapter) + + allow(Listen::Adapter::Polling).to receive(:new).with(adapter_config). + and_return(adapter) + + allow(config).to receive(:adapter_select_options). + and_return(select_options) + + allow(config).to receive(:adapter_instance_options). + and_return(adapter_options) + + allow(config).to receive(:min_delay_between_events). + and_return(config_min_delay_between_events) + end + + describe '#initialize' do + context 'with config' do + it 'sets up an adapter class' do + expect(adapter_class).to receive(:new). + with(adapter_config).and_return(adapter) + + subject + end + end + end + + describe '#start' do + it 'starts the adapter' do + expect(adapter).to receive(:start) + subject.start + end + end + + describe '#stop' do + it 'stops the adapter' do + # TODO: does nothing for now + subject.stop + end + end +end From 1f5d17aa8c25720944abcfd281ceac0c8400acef Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:28:46 +0200 Subject: [PATCH 15/47] add Adapter::Config class --- lib/listen/adapter/config.rb | 21 ++++++ spec/lib/listen/adapter/config_spec.rb | 97 ++++++++++++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100644 lib/listen/adapter/config.rb create mode 100644 spec/lib/listen/adapter/config_spec.rb diff --git a/lib/listen/adapter/config.rb b/lib/listen/adapter/config.rb new file mode 100644 index 00000000..a5bc6222 --- /dev/null +++ b/lib/listen/adapter/config.rb @@ -0,0 +1,21 @@ +module Listen + module Adapter + class Config + attr_reader :directories + attr_reader :silencer + attr_reader :queue + attr_reader :adapter_options + + def initialize(directories, queue, silencer, adapter_options) + # TODO: fix (flatten, array, compact?) + @directories = directories.map do |directory| + Pathname.new(directory.to_s).realpath + end + + @silencer = silencer + @queue = queue + @adapter_options = adapter_options + end + end + end +end diff --git a/spec/lib/listen/adapter/config_spec.rb b/spec/lib/listen/adapter/config_spec.rb new file mode 100644 index 00000000..539cb2a4 --- /dev/null +++ b/spec/lib/listen/adapter/config_spec.rb @@ -0,0 +1,97 @@ +require 'listen/adapter/config' + +RSpec.describe Listen::Adapter::Config do + let(:directories) { [path1, path2] } + let(:queue) { instance_double(Queue) } + let(:silencer) { instance_double(Listen::Silencer) } + + # NOTE: defaults are handled later in Listen::Options + let(:adapter_options) { { latency: 1.234 } } + + subject do + described_class.new(directories, queue, silencer, adapter_options) + end + + # Here's what may be passed to initializer + let(:path1) { fake_path('/real/path1', realpath: real_path1) } + let(:path2) { fake_path('/real/path2', realpath: real_path2) } + + let(:symlinked_dir1) { fake_path('symlinked_dir1', realpath: real_path1) } + let(:symlinked_dir2) { fake_path('symlinked_dir1', realpath: real_path2) } + + # Here's what expected to be returned (just so that realpath() calls return + # something useful) + let(:real_path1) { fake_path('/real/path1') } + let(:real_path2) { fake_path('/real/path2') } + + before do + allow(Pathname).to receive(:new) do |*args| + fail "unstubbed Pathname.new(#{args.map(&:inspect) * ','})" + end + + allow(Pathname).to receive(:new).with('/real/path1').and_return(path1) + allow(Pathname).to receive(:new).with('/real/path2').and_return(path2) + + allow(Pathname).to receive(:new).with(path1).and_return(path1) + allow(Pathname).to receive(:new).with(path2).and_return(path2) + + allow(Pathname).to receive(:new).with('symlinked_dir1'). + and_return(symlinked_dir1) + + allow(Pathname).to receive(:new).with('symlinked_dir2'). + and_return(symlinked_dir2) + end + + describe '#initialize' do + context 'with directories as array' do + context 'with strings for directories' do + context 'when already resolved' do + let(:directories) { ['/real/path1', '/real/path2'] } + it 'returns array of pathnames' do + expect(subject.directories).to eq([real_path1, real_path2]) + end + end + + context 'when not resolved' do + let(:directories) { ['symlinked_dir1', 'symlinked_dir2'] } + it 'returns array of resolved pathnames' do + expect(subject.directories).to eq([real_path1, real_path2]) + end + end + end + + context 'with Pathnames for directories' do + let(:directories) { [path1, path2] } + it 'returns array of pathnames' do + expect(subject.directories).to eq([real_path1, real_path2]) + end + end + end + + context 'with directories as messy array' do + pending 'implement me' + end + + context 'with no directories' do + pending 'implement me' + end + end + + describe '#adapter_options' do + it 'provides a set of adapter_specific options' do + expect(subject.adapter_options).to eq(latency: 1.234) + end + end + + describe '#queue' do + it 'provides a direct queue for filesystem events' do + expect(subject.queue).to eq(queue) + end + end + + describe '#silencer' do + it 'provides a silencer object' do + expect(subject.silencer).to eq(silencer) + end + end +end From 43f4e390b3b4292893f775885d6869e4dd2ad67b Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:29:08 +0200 Subject: [PATCH 16/47] add Listener::Config class --- lib/listen/listener/config.rb | 43 +++++++++++++++++++++++++ spec/lib/listen/listener/config_spec.rb | 27 ++++++++++++++++ 2 files changed, 70 insertions(+) create mode 100644 lib/listen/listener/config.rb create mode 100644 spec/lib/listen/listener/config_spec.rb diff --git a/lib/listen/listener/config.rb b/lib/listen/listener/config.rb new file mode 100644 index 00000000..ced287cd --- /dev/null +++ b/lib/listen/listener/config.rb @@ -0,0 +1,43 @@ +module Listen + class Listener + class Config + DEFAULTS = { + debug: false, # TODO: is this broken? + latency: nil, + wait_for_delay: nil, # NOTE: should be provided by adapter if possible + force_polling: false, + relative: false, + polling_fallback_message: nil + } + + def initialize(opts) + @options = DEFAULTS.merge(opts) + @relative = @options[:relative] + @min_delay_between_events = @options[:wait_for_delay] + @silencer_rules = @options # silencer will extract what it needs + end + + def relative? + @relative + end + + def min_delay_between_events + @min_delay_between_events + end + + def silencer_rules + @silencer_rules + end + + def adapter_instance_options(klass) + valid_keys = klass.const_get('DEFAULTS').keys + Hash[@options.select { |key, _| valid_keys.include?(key) }] + end + + def adapter_select_options + valid_keys = %w(force_polling polling_fallback_message).map(&:to_sym) + Hash[@options.select { |key, _| valid_keys.include?(key) }] + end + end + end +end diff --git a/spec/lib/listen/listener/config_spec.rb b/spec/lib/listen/listener/config_spec.rb new file mode 100644 index 00000000..ec92a867 --- /dev/null +++ b/spec/lib/listen/listener/config_spec.rb @@ -0,0 +1,27 @@ +require 'listen/listener/config' +RSpec.describe Listen::Listener::Config do + describe 'options' do + context 'custom options' do + subject do + described_class.new( + latency: 1.234, + wait_for_delay: 0.85, + force_polling: true, + relative: true) + end + + it 'extracts adapter options' do + klass = Class.new do + DEFAULTS = { latency: 5.4321 } + end + expected = { latency: 1.234 } + expect(subject.adapter_instance_options(klass)).to eq(expected) + end + + it 'extract adapter selecting options' do + expected = { force_polling: true, polling_fallback_message: nil } + expect(subject.adapter_select_options).to eq(expected) + end + end + end +end From a79969bf95e1a1da55892799f6c599808c7eabc4 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:29:33 +0200 Subject: [PATCH 17/47] allow using installed Listen in dev --- Gemfile | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 5619019f..8c7eb79d 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,13 @@ source 'https://rubygems.org' -gemspec development_group: :gem_build_tools +# Create this file to use pristine/installed version of Listen for development +use_installed = "./use_installed_guard" +if File.exist?(use_installed) + STDERR.puts "WARNING: using installed version of Listen for development" \ + " (remove #{use_installed} file to use local version)" +else + gemspec development_group: :gem_build_tools +end require 'rbconfig' From f8f406e96736d2c4cd799b053653827c0ed0795f Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:32:06 +0200 Subject: [PATCH 18/47] Guardfile: run acceptance specs after normal specs --- Guardfile | 14 ++++++++++++-- spec/acceptance/listen_spec.rb | 2 +- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/Guardfile b/Guardfile index 3048604f..1b47df5c 100644 --- a/Guardfile +++ b/Guardfile @@ -1,8 +1,8 @@ ignore(%r{spec/\.fixtures/}) group :specs, halt_on_fail: true do - guard :rspec, cmd: 'bundle exec rspec', failed_mode: :keep do - watch(%r{^spec/.+_spec\.rb$}) + guard :rspec, cmd: 'bundle exec rspec -t ~acceptance', failed_mode: :keep, all_after_pass: true do + watch(%r{^spec/lib/.+_spec\.rb$}) watch(%r{^lib/(.+)\.rb$}) { |m| "spec/lib/#{m[1]}_spec.rb" } watch(%r{^spec/support/*}) { 'spec' } watch('spec/spec_helper.rb') { 'spec' } @@ -13,4 +13,14 @@ group :specs, halt_on_fail: true do watch(%r{(?:.+/)?\.rubocop\.yml$}) { |m| File.dirname(m[0]) } watch(%r{(?:.+/)?\.rubocop_todo\.yml$}) { |m| File.dirname(m[0]) } end + + # TODO: guard rspec should have a configurable file for this to work + # TODO: also split up Rakefile + guard :rspec, cmd: 'bundle exec rspec -t acceptance', failed_mode: :keep, all_after_pass: true do + watch(%r{^spec/lib/.+_spec\.rb$}) + watch(%r{^lib/(.+)\.rb$}) { |m| "spec/lib/#{m[1]}_spec.rb" } + watch(%r{^spec/support/*}) { 'spec' } + watch('spec/spec_helper.rb') { 'spec' } + watch(%r{^spec/acceptance/.+_spec\.rb$}) + end end diff --git a/spec/acceptance/listen_spec.rb b/spec/acceptance/listen_spec.rb index 396f5d03..27888250 100644 --- a/spec/acceptance/listen_spec.rb +++ b/spec/acceptance/listen_spec.rb @@ -1,6 +1,6 @@ # encoding: UTF-8 -RSpec.describe 'Listen' do +RSpec.describe 'Listen', acceptance: true do let(:base_options) { { wait_for_delay: 0.1, latency: 0.1 } } let(:polling_options) { {} } let(:options) { {} } From c31bb8764bc15b46e54077008a96037cb9f5f0f5 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:32:55 +0200 Subject: [PATCH 19/47] clear up logging functionality --- lib/listen.rb | 30 +++++++++++----------------- lib/listen/change.rb | 23 ++++++++++++---------- lib/listen/directory.rb | 4 ++-- lib/listen/internals/logging.rb | 35 --------------------------------- lib/listen/logger.rb | 15 ++++++++++++++ spec/acceptance/listen_spec.rb | 2 +- spec/spec_helper.rb | 4 +++- 7 files changed, 45 insertions(+), 68 deletions(-) delete mode 100644 lib/listen/internals/logging.rb diff --git a/lib/listen.rb b/lib/listen.rb index 8bd88b3c..cde7d130 100644 --- a/lib/listen.rb +++ b/lib/listen.rb @@ -4,26 +4,18 @@ require 'listen/internals/thread_pool' -# Set up logging by default first time file is requried +# Always set up logging by default first time file is requried # -Listen.logger ||= Logger.new(STDERR) - -if Listen.logger - debugging = ENV['LISTEN_GEM_DEBUGGING'] - - Listen.logger.level = - case debugging.to_s - when /2/ - ::Logger::DEBUG - when /true|yes|1/i - ::Logger::INFO - else - ::Logger::ERROR - end - - Listen.logger.info "Listen loglevel set to: #{Listen.logger.level}" - Listen.logger.info "Listen version: #{Listen::VERSION}" -end +# NOTE: If you need to clear the logger completely, do so *after* +# requiring this file. If you need to set a custom logger, +# require the listen/logger file and set the logger before requiring +# this file. +Listen.setup_default_logger_if_unset + +# Won't print anything by default because of level - unless you've set +# LISTEN_GEM_DEBUGGING or provided your own logger with a high enough level +Listen::Logger.info "Listen loglevel set to: #{Listen.logger.level}" +Listen::Logger.info "Listen version: #{Listen::VERSION}" module Listen class << self diff --git a/lib/listen/change.rb b/lib/listen/change.rb index 76bbc3e1..4c97691a 100644 --- a/lib/listen/change.rb +++ b/lib/listen/change.rb @@ -36,14 +36,16 @@ def change(type, rel_path, options) cookie = options[:cookie] if !cookie && config.silenced?(rel_path, type) - _log :debug, "(silenced): #{rel_path.inspect}" + Listen::Logger.debug { "(silenced): #{rel_path.inspect}" } return end path = watched_dir + rel_path - log_details = options[:silence] && 'recording' || change || 'unknown' - _log :debug, "#{log_details}: #{type}:#{path} (#{options.inspect})" + Listen::Logger.debug do + log_details = options[:silence] && 'recording' || change || 'unknown' + "#{log_details}: #{type}:#{path} (#{options.inspect})" + end if change # TODO: move this to Listener to avoid overhead @@ -61,18 +63,19 @@ def change(type, rel_path, options) config.queue(:file, change, watched_dir, rel_path) end end - rescue RuntimeError - _log :error, format('Change#change crashed %s:%s', $ERROR_INFO.inspect, - $ERROR_POSITION * "\n") + rescue RuntimeError => ex + msg = format( + '%s#%s crashed %s:%s', + self.class, + __method__, + exinspect, + ex.backtrace * "\n") + Listen::Logger.error(msg) raise end private attr_reader :config - - def _log(type, message) - Listen::Logger.send(type, message) - end end end diff --git a/lib/listen/directory.rb b/lib/listen/directory.rb index 126c5bc3..f850d558 100644 --- a/lib/listen/directory.rb +++ b/lib/listen/directory.rb @@ -13,7 +13,7 @@ def self.scan(fs_change, rel_path, options) path = dir + rel_path current = Set.new(path.children) - _log(:debug) do + Listen::Logger.debug do format('%s: %s(%s): %s -> %s', (options[:silence] ? 'Recording' : 'Scanning'), rel_path, options.inspect, previous.inspect, current.inspect) @@ -40,7 +40,7 @@ def self.scan(fs_change, rel_path, options) _async_changes(fs_change, path, previous, options) _change(fs_change, :file, rel_path, options) rescue - _log(:warn) do + Listen::Logger.warn do format('scan DIED: %s:%s', $ERROR_INFO, $ERROR_POSITION * "\n") end raise diff --git a/lib/listen/internals/logging.rb b/lib/listen/internals/logging.rb deleted file mode 100644 index 51eef4a7..00000000 --- a/lib/listen/internals/logging.rb +++ /dev/null @@ -1,35 +0,0 @@ -require 'listen/logger' - -module Listen - module Internals - module Logging - def _info(*args, &block) - _log(:info, *args, &block) - end - - def _warn(*args, &block) - _log(:warn, *args, &block) - end - - def _debug(*args, &block) - _log(:debug, *args, &block) - end - - def _log(*args, &block) - if block - Listen::Logger.send(*args, block.call) - else - Listen::Logger.send(*args) - end - end - - def _format_error(fmt) - format(fmt, $ERROR_INFO, ", Backtrace: \n" + $ERROR_POSITION * "\n") - end - - def _error_exception(fmt) - _log :error, _format_error(fmt) - end - end - end -end diff --git a/lib/listen/logger.rb b/lib/listen/logger.rb index da451738..4eac8020 100644 --- a/lib/listen/logger.rb +++ b/lib/listen/logger.rb @@ -7,6 +7,21 @@ def self.logger=(logger) @logger = logger end + def self.setup_default_logger_if_unset + self.logger ||= ::Logger.new(STDERR).tap do |logger| + debugging = ENV['LISTEN_GEM_DEBUGGING'] + logger.level = + case debugging.to_s + when /2/ + ::Logger::DEBUG + when /true|yes|1/i + ::Logger::INFO + else + ::Logger::ERROR + end + end + end + class Logger %i(fatal error warn info debug).each do |meth| define_singleton_method(meth) do |*args, &block| diff --git a/spec/acceptance/listen_spec.rb b/spec/acceptance/listen_spec.rb index 27888250..21119b45 100644 --- a/spec/acceptance/listen_spec.rb +++ b/spec/acceptance/listen_spec.rb @@ -21,7 +21,7 @@ let(:wrapper) { setup_listener(all_options, callback) } it 'warns the backtrace' do - expect(Listen.logger).to receive(:error). + expect(Listen::Logger).to receive(:error). with(/exception while processing events: foo.*Backtrace:/) wrapper.listen { touch 'file.rb' } end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 428ee32c..0a848af6 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -1,5 +1,8 @@ +# TODO: reduce requires everwhere and be more strict about it require 'listen' +Listen.logger.level = Logger::WARN unless ENV['LISTEN_GEM_DEBUGGING'] + require 'listen/internals/thread_pool' def ci? @@ -44,7 +47,6 @@ def fake_path(str, options = {}) end Thread.abort_on_exception = true -Listen.logger.level = Logger::DEBUG RSpec.configuration.before(:each) do Listen::Internals::ThreadPool.stop From 666c2bb34515e2b054582b923528583937624cf7 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:33:54 +0200 Subject: [PATCH 20/47] rework Adapter to use Record and Config --- lib/listen/adapter/base.rb | 97 ++++++++++++++++++++++++-------------- 1 file changed, 61 insertions(+), 36 deletions(-) diff --git a/lib/listen/adapter/base.rb b/lib/listen/adapter/base.rb index 247277c5..7819810a 100644 --- a/lib/listen/adapter/base.rb +++ b/lib/listen/adapter/base.rb @@ -1,4 +1,6 @@ require 'listen/options' +require 'listen/record' +require 'listen/change' module Listen module Adapter @@ -8,23 +10,18 @@ class Base # TODO: only used by tests DEFAULTS = {} - def initialize(opts) - @configured = nil - options = opts.dup - @mq = options.delete(:mq) - @directories = options.delete(:directories) + attr_reader :config - Array(@directories).each do |dir| - next if dir.is_a?(Pathname) - fail ArgumentError, "not a Pathname: #{dir.inspect}" - end + def initialize(config) + @started = false + @config = config + + @configured = nil - # TODO: actually use this in every adapter - @recursion = options.delete(:recursion) - @recursion = true if @recursion.nil? + fail 'No directories to watch!' if config.directories.empty? defaults = self.class.const_get('DEFAULTS') - @options = Listen::Options.new(options, defaults) + @options = Listen::Options.new(config.adapter_options, defaults) rescue _log_exception 'adapter config failed: %s:%s called from: %s', caller raise @@ -32,52 +29,82 @@ def initialize(opts) # TODO: it's a separate method as a temporary workaround for tests def configure - return if @configured + if @configured + _log(:warn, 'Adapter already configured!') + return + end + @configured = true @callbacks ||= {} - @directories.each do |dir| - unless dir.is_a?(Pathname) - fail ArgumentError, "not a Pathname: #{dir.inspect}" - end - + config.directories.each do |dir| callback = @callbacks[dir] || lambda do |event| _process_event(dir, event) end @callbacks[dir] = callback _configure(dir, &callback) end + + @snapshots ||= {} + # TODO: separate config per directory (some day maybe) + change_config = Change::Config.new(config.queue, config.silencer) + config.directories.each do |dir| + record = Record.new(dir) + snapshot = Change.new(change_config, record) + @snapshots[dir] = snapshot + end + end + + def started? + @started end def start configure + + if started? + _log(:warn, 'Adapter already started!') + return + end + + @started = true + calling_stack = caller.dup Listen::Internals::ThreadPool.add do begin + @snapshots.values.each do |snapshot| + _timed('Record.build()') { snapshot.record.build } + end _run rescue - msg = "run() in thread failed: %s:\n"\ - " %s\n\ncalled from:\n %s", calling_stack - - _log_exception(msg) - raise + msg = 'run() in thread failed: %s:\n'\ + ' %s\n\ncalled from:\n %s' + _log_exception(msg, calling_stack) + raise # for unit tests mostly end end end - def self.local_fs? - true - end - def self.usable? const_get('OS_REGEXP') =~ RbConfig::CONFIG['target_os'] end private + def _timed(title) + start = Time.now.to_f + yield + diff = Time.now.to_f - start + Listen::Logger.info format('%s: %.05f seconds', title, diff) + rescue + Listen::Logger.warn "#{title} crashed: #{$ERROR_INFO.inspect}" + raise + end + + # TODO: allow backend adapters to pass specific invalidation objects + # e.g. Darwin -> DirRescan, INotify -> MoveScan, etc. def _queue_change(type, dir, rel_path, options) - # TODO: temporary workaround to remove dependency on Change - @mq.send(:_queue_raw_change, type, dir, rel_path, options) + @snapshots[dir].invalidate(type, rel_path, options) end def _log(*args, &block) @@ -85,20 +112,18 @@ def _log(*args, &block) end def _log_exception(msg, caller_stack) - _log :error, format( + formatted = format( msg, $ERROR_INFO, $ERROR_POSITION * "\n", caller_stack * "\n" ) + + _log(:error, formatted) end def self._log(*args, &block) - if block - Listen::Logger.send(*args, block.call) - else - Listen::Logger.send(*args) - end + Listen::Logger.send(*args, &block) end end end From 29dc723c75b3d231762d691ebff4e4f468d8e18b Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:34:24 +0200 Subject: [PATCH 21/47] make wait_for_delay default adapter specific --- lib/listen/adapter/linux.rb | 3 ++- lib/listen/adapter/polling.rb | 2 +- spec/acceptance/listen_spec.rb | 2 +- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/listen/adapter/linux.rb b/lib/listen/adapter/linux.rb index 17cc4399..56c0deb8 100644 --- a/lib/listen/adapter/linux.rb +++ b/lib/listen/adapter/linux.rb @@ -12,7 +12,8 @@ class Linux < Base :delete, :move, :close_write - ] + ], + wait_for_delay: 0.1 } private diff --git a/lib/listen/adapter/polling.rb b/lib/listen/adapter/polling.rb index c677588c..b28d44ad 100644 --- a/lib/listen/adapter/polling.rb +++ b/lib/listen/adapter/polling.rb @@ -8,7 +8,7 @@ module Adapter class Polling < Base OS_REGEXP = // # match every OS - DEFAULTS = { latency: 1.0 } + DEFAULTS = { latency: 1.0, wait_for_delay: 0.01 } private diff --git a/spec/acceptance/listen_spec.rb b/spec/acceptance/listen_spec.rb index 21119b45..70cec336 100644 --- a/spec/acceptance/listen_spec.rb +++ b/spec/acceptance/listen_spec.rb @@ -1,7 +1,7 @@ # encoding: UTF-8 RSpec.describe 'Listen', acceptance: true do - let(:base_options) { { wait_for_delay: 0.1, latency: 0.1 } } + let(:base_options) { { latency: 0.1 } } let(:polling_options) { {} } let(:options) { {} } let(:all_options) { base_options.merge(polling_options).merge(options) } From 309a3f64e7f440fd433a89ed1971fc5e2fa91777 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:34:47 +0200 Subject: [PATCH 22/47] fix Linux adapter specs --- lib/listen/adapter/linux.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/listen/adapter/linux.rb b/lib/listen/adapter/linux.rb index 56c0deb8..def22629 100644 --- a/lib/listen/adapter/linux.rb +++ b/lib/listen/adapter/linux.rb @@ -27,8 +27,8 @@ class Linux < Base EOS def _configure(directory, &callback) - require 'rb-inotify' - @worker ||= INotify::Notifier.new + Kernel.require 'rb-inotify' + @worker ||= ::INotify::Notifier.new @worker.watch(directory.to_s, *options.events, &callback) rescue Errno::ENOSPC abort(INOTIFY_LIMIT_MESSAGE) From 4e825eeb60a43193a00d1067a1c7a1b7714037f9 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:35:08 +0200 Subject: [PATCH 23/47] add TODO for polling --- lib/listen/adapter/polling.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/listen/adapter/polling.rb b/lib/listen/adapter/polling.rb index b28d44ad..ae353aa6 100644 --- a/lib/listen/adapter/polling.rb +++ b/lib/listen/adapter/polling.rb @@ -23,6 +23,7 @@ def _run @polling_callbacks.each do |callback| callback.call(nil) nap_time = options.latency - (Time.now.to_f - start) + # TODO: warn if nap_time is negative (polling too slow) sleep(nap_time) if nap_time > 0 end end From f5bf958aaac93b74c08e9890f50ab8458230bc1f Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:35:30 +0200 Subject: [PATCH 24/47] add TODOs for Change --- lib/listen/change.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/listen/change.rb b/lib/listen/change.rb index 4c97691a..521073fe 100644 --- a/lib/listen/change.rb +++ b/lib/listen/change.rb @@ -2,7 +2,9 @@ require 'listen/directory' module Listen + # TODO: rename to Snapshot class Change + # TODO: test this class for coverage class Config attr_reader :listener def initialize(listener) From 3ae5098371de24840f52404505076491cb8717b6 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:36:23 +0200 Subject: [PATCH 25/47] allow Change to use Silencer and Queue directly --- lib/listen/change.rb | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/lib/listen/change.rb b/lib/listen/change.rb index 521073fe..2899dec8 100644 --- a/lib/listen/change.rb +++ b/lib/listen/change.rb @@ -6,21 +6,17 @@ module Listen class Change # TODO: test this class for coverage class Config - attr_reader :listener - def initialize(listener) - @listener = listener + def initialize(queue, silencer) + @queue = queue + @silencer = silencer end def silenced?(path, type) - listener.silencer.silenced?(Pathname(path), type) - end - - def record_for(directory) - listener.record_for(directory) + @silencer.silenced?(Pathname(path), type) end def queue(*args) - listener.queue(*args) + @queue << args end end From 1347ceb40dadef4222611eab8b3258b055bfde74 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:37:08 +0200 Subject: [PATCH 26/47] rename Change#change to #invalidate --- lib/listen/change.rb | 3 ++- spec/lib/listen/change_spec.rb | 19 +++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lib/listen/change.rb b/lib/listen/change.rb index 2899dec8..2c37713c 100644 --- a/lib/listen/change.rb +++ b/lib/listen/change.rb @@ -27,7 +27,8 @@ def initialize(config, record) @record = record end - def change(type, rel_path, options) + # Invalidate some part of the snapshot/record (dir, file, subtree, etc.) + def invalidate(type, rel_path, options) watched_dir = Pathname.new(record.root) change = options[:change] diff --git a/spec/lib/listen/change_spec.rb b/spec/lib/listen/change_spec.rb index 5b7fc00e..95598c4c 100644 --- a/spec/lib/listen/change_spec.rb +++ b/spec/lib/listen/change_spec.rb @@ -20,9 +20,8 @@ context 'with build options' do it 'calls still_building! on record' do allow(config).to receive(:queue) - allow(config).to receive(:record_for).with(dir).and_return(record) allow(Listen::File).to receive(:change) - subject.change(:file, 'file.rb', build: true) + subject.invalidate(:file, 'file.rb', build: true) end end @@ -32,13 +31,13 @@ expect(config).to receive(:queue). with(:file, :modified, Pathname.new('/dir'), 'file.rb', {}) - subject.change(:file, 'file.rb', change: :modified) + subject.invalidate(:file, 'file.rb', change: :modified) end it "doesn't notify to listener if path is silenced" do expect(config).to receive(:silenced?).and_return(true) expect(config).to_not receive(:queue) - subject.change(:file, 'file.rb', change: :modified) + subject.invalidate(:file, 'file.rb', change: :modified) end end @@ -46,7 +45,7 @@ it 'calls Listen::File#change' do expect(Listen::File).to receive(:change).with(record, 'file.rb') - subject.change(:file, 'file.rb', {}) + subject.invalidate(:file, 'file.rb', {}) end it "doesn't call Listen::File#change if path is silenced" do @@ -54,7 +53,7 @@ with('file.rb', :file).and_return(true) expect(Listen::File).to_not receive(:change) - subject.change(:file, 'file.rb', {}) + subject.invalidate(:file, 'file.rb', {}) end context 'that returns a change' do @@ -65,13 +64,13 @@ expect(config).to receive(:queue). with(:file, :modified, Pathname.new('/dir'), 'file.rb') - subject.change(:file, 'file.rb', {}) + subject.invalidate(:file, 'file.rb', {}) end context 'silence option' do it 'notifies change to listener' do expect(config).to_not receive(:queue) - subject.change(:file, 'file.rb', silence: true) + subject.invalidate(:file, 'file.rb', silence: true) end end end @@ -82,7 +81,7 @@ it "doesn't notifies no change" do expect(config).to_not receive(:queue) - subject.change(:file, 'file.rb', {}) + subject.invalidate(:file, 'file.rb', {}) end end end @@ -95,7 +94,7 @@ expect(Listen::Directory).to receive(:scan). with(subject, 'dir1', dir_options) - subject.change(:dir, 'dir1', dir_options) + subject.invalidate(:dir, 'dir1', dir_options) end end end From c3ea159f4e962c5d3a552dd38690ba1b6ca9c5cf Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:37:53 +0200 Subject: [PATCH 27/47] remove TODO --- lib/listen/change.rb | 2 -- 1 file changed, 2 deletions(-) diff --git a/lib/listen/change.rb b/lib/listen/change.rb index 2c37713c..b73028d7 100644 --- a/lib/listen/change.rb +++ b/lib/listen/change.rb @@ -47,8 +47,6 @@ def invalidate(type, rel_path, options) end if change - # TODO: move this to Listener to avoid overhead - # from caller options = cookie ? { cookie: cookie } : {} config.queue(type, change, watched_dir, rel_path, options) else From 8c3899cab7b1441bbab35330978e7cee64be3527 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:38:27 +0200 Subject: [PATCH 28/47] use processing? instead of nice but ambiguous listen? --- lib/listen/cli.rb | 2 +- spec/lib/listen/cli_spec.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/listen/cli.rb b/lib/listen/cli.rb index ed69c9af..851f063c 100644 --- a/lib/listen/cli.rb +++ b/lib/listen/cli.rb @@ -59,7 +59,7 @@ def start listener.start - sleep 0.5 while listener.listen? + sleep 0.5 while listener.processing? end end end diff --git a/spec/lib/listen/cli_spec.rb b/spec/lib/listen/cli_spec.rb index 5606a631..63ee7373 100644 --- a/spec/lib/listen/cli_spec.rb +++ b/spec/lib/listen/cli_spec.rb @@ -100,7 +100,7 @@ allow(logger).to receive(:info) allow(listener).to receive(:start) - allow(listener).to receive(:listen?).and_return false + allow(listener).to receive(:processing?).and_return false end it 'passes relative option to Listen' do From eda455d7f72e131a9d2f6dbaffa0b0df2caeeabf Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:39:15 +0200 Subject: [PATCH 29/47] cosmetic changes in Directory --- lib/listen/directory.rb | 25 +++++++++++++------------ 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/lib/listen/directory.rb b/lib/listen/directory.rb index f850d558..643d959f 100644 --- a/lib/listen/directory.rb +++ b/lib/listen/directory.rb @@ -1,9 +1,10 @@ require 'set' module Listen + # TODO: refactor (turn it into a normal object, cache the stat, etc) class Directory - def self.scan(fs_change, rel_path, options) - record = fs_change.record + def self.scan(snapshot, rel_path, options) + record = snapshot.record dir = Pathname.new(record.root) previous = record.dir_entries(rel_path) @@ -22,23 +23,23 @@ def self.scan(fs_change, rel_path, options) current.each do |full_path| type = full_path.directory? ? :dir : :file item_rel_path = full_path.relative_path_from(dir).to_s - _change(fs_change, type, item_rel_path, options) + _change(snapshot, type, item_rel_path, options) end # TODO: this is not tested properly previous = previous.reject { |entry, _| current.include? path + entry } - _async_changes(fs_change, Pathname.new(rel_path), previous, options) + _async_changes(snapshot, Pathname.new(rel_path), previous, options) rescue Errno::ENOENT, Errno::EHOSTDOWN record.unset_path(rel_path) - _async_changes(fs_change, Pathname.new(rel_path), previous, options) + _async_changes(snapshot, Pathname.new(rel_path), previous, options) rescue Errno::ENOTDIR # TODO: path not tested record.unset_path(rel_path) - _async_changes(fs_change, path, previous, options) - _change(fs_change, :file, rel_path, options) + _async_changes(snapshot, path, previous, options) + _change(snapshot, :file, rel_path, options) rescue Listen::Logger.warn do format('scan DIED: %s:%s', $ERROR_INFO, $ERROR_POSITION * "\n") @@ -46,24 +47,24 @@ def self.scan(fs_change, rel_path, options) raise end - def self._async_changes(fs_change, path, previous, options) + def self._async_changes(snapshot, path, previous, options) fail "Not a Pathname: #{path.inspect}" unless path.respond_to?(:children) previous.each do |entry, data| # TODO: this is a hack with insufficient testing type = data.key?(:mtime) ? :file : :dir rel_path_s = (path + entry).to_s - _change(fs_change, type, rel_path_s, options) + _change(snapshot, type, rel_path_s, options) end end - def self._change(fs_change, type, path, options) - return fs_change.change(type, path, options) if type == :dir + def self._change(snapshot, type, path, options) + return snapshot.invalidate(type, path, options) if type == :dir # Minor param cleanup for tests # TODO: use a dedicated Event class opts = options.dup opts.delete(:recursive) - fs_change.change(type, path, opts) + snapshot.invalidate(type, path, opts) end def self._log(type, &block) From bee701b3625a09b2ba8fdc0328026486a2907c06 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:40:12 +0200 Subject: [PATCH 30/47] try to be a bit smarter at detecting type This was an attempt to fix some race conditions in tests, though ideally Record and Change should be improved to more accurately contain a consistent filesystem snapshot. --- lib/listen/directory.rb | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/lib/listen/directory.rb b/lib/listen/directory.rb index 643d959f..b57e1480 100644 --- a/lib/listen/directory.rb +++ b/lib/listen/directory.rb @@ -21,7 +21,7 @@ def self.scan(snapshot, rel_path, options) end current.each do |full_path| - type = full_path.directory? ? :dir : :file + type = detect_type(full_path) item_rel_path = full_path.relative_path_from(dir).to_s _change(snapshot, type, item_rel_path, options) end @@ -67,11 +67,14 @@ def self._change(snapshot, type, path, options) snapshot.invalidate(type, path, opts) end - def self._log(type, &block) - return unless Listen.logger - Listen.logger.send(type) do - block.call - end + def self.detect_type(full_path) + # TODO: should probably check record first + stat = ::File.lstat(full_path.to_s) + stat.directory? ? :dir : :file + rescue Errno::ENOENT + # TODO: ok, it should really check the record here + # report as dir for scanning + :dir end end end From bfd49cb42b869bd69f13b28dbec7fd14e951aed2 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:42:45 +0200 Subject: [PATCH 31/47] allow ThreadPool to work with stubs --- lib/listen/internals/thread_pool.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/listen/internals/thread_pool.rb b/lib/listen/internals/thread_pool.rb index bb276592..b1fe7567 100644 --- a/lib/listen/internals/thread_pool.rb +++ b/lib/listen/internals/thread_pool.rb @@ -10,6 +10,7 @@ def self.add(&block) def self.stop return unless @threads ||= nil + return if @threads.empty? # return to avoid using possibly stubbed Queue killed = Queue.new killed << @threads.pop.kill until @threads.empty? From a176745a275f91c992d35c5700461504c07d9498 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:43:42 +0200 Subject: [PATCH 32/47] rework and reduce Listener class --- lib/listen/listener.rb | 225 ++++++++-------------------------- lib/listen/listener/config.rb | 6 +- 2 files changed, 56 insertions(+), 175 deletions(-) diff --git a/lib/listen/listener.rb b/lib/listen/listener.rb index 52ea22cf..86c6c5e8 100644 --- a/lib/listen/listener.rb +++ b/lib/listen/listener.rb @@ -1,34 +1,26 @@ -require 'pathname' +require 'English' require 'listen/version' -require 'listen/adapter' -require 'listen/change' -require 'listen/record' + +require 'listen/backend' require 'listen/silencer' require 'listen/silencer/controller' require 'listen/queue_optimizer' -require 'English' - -require 'listen/internals/logging' - require 'listen/fsm' -require 'listen/event_processor' +require 'listen/event/loop' +require 'listen/event/queue' +require 'listen/event/config' + +require 'listen/listener/config' module Listen class Listener include Listen::FSM - attr_accessor :block - - attr_reader :silencer - - # TODO: deprecate - attr_reader :options, :directories - # Initializes the directories listener. # # @param [String] directory the directories to listen to @@ -39,82 +31,69 @@ class Listener # @yieldparam [Array] added the list of added files # @yieldparam [Array] removed the list of removed files # - def initialize(*args, &block) - @options = _init_options(args.last.is_a?(Hash) ? args.pop : {}) + def initialize(*dirs, &block) + options = dirs.last.is_a?(Hash) ? dirs.pop : {} - @silencer = Silencer.new - @silencer_controller = Silencer::Controller.new(@silencer, @options) + @config = Config.new(options) - @directories = args.flatten.map { |path| Pathname.new(path).realpath } - @event_queue = Queue.new - @block = block + eq_config = Event::Queue::Config.new(@config.relative?) + queue = Event::Queue.new(eq_config) { @processor.wakeup_on_event } - change_config = Listen::Change::Config.new(self) - changes = @directories.map do |dir| - [ - dir.to_s, - Change.new(change_config, Record.new(dir)) - ] - end - @fs_changes = Hash[changes] + silencer = Silencer.new + rules = @config.silencer_rules + @silencer_controller = Silencer::Controller.new(silencer, rules) - adapter_options = { mq: self, directories: directories } + @backend = Backend.new(dirs, queue, silencer, @config) - # TODO: refactor - valid_adapter_options = _adapter_class.const_get(:DEFAULTS).keys - valid_adapter_options.each do |key| - adapter_options.merge!(key => options[key]) if options.key?(key) - end + optimizer_config = QueueOptimizer::Config.new(@backend, silencer) - @adapter = Adapter.select(options).new(adapter_options) + pconfig = Event::Config.new( + self, + queue, + QueueOptimizer.new(optimizer_config), + @backend.min_delay_between_events, + &block) - optimizer_config = QueueOptimizer::Config.new(@adapter.class, @silencer) - @queue_optimizer = QueueOptimizer.new(optimizer_config) + @processor = Event::Loop.new(pconfig) - transition :stopped + super() # FSM end default_state :initializing - state :initializing, to: :stopped - state :paused, to: [:processing, :stopped] + state :initializing, to: :backend_started + + state :backend_started, to: [:frontend_ready] do + backend.start + end - state :stopped, to: [:processing] do - _stop_wait_thread + state :frontend_ready, to: [:processing_events] do + processor.setup end - state :processing, to: [:paused, :stopped] do - if wait_thread # means - was paused - _wakeup_wait_thread - else - self.last_queue_event_time = nil - _start_wait_thread - - begin - start = Time.now.to_f - # Note: make sure building is finished before starting adapter (for - # consistent results both in specs and normal usage) - fs_changes.values.map(&:record).map(&:build) - Listen::Logger.info "Record.build(): #{Time.now.to_f - start} seconds" - rescue - Listen::Logger.warn "build crashed: #{$ERROR_INFO.inspect}" - raise - end - - @adapter.start - end + state :processing_events, to: [:paused, :stopped] do + processor.resume + end + + state :paused, to: [:processing_events, :stopped] do + processor.pause + end + + state :stopped, to: [:backend_started] do + backend.stop # should be before processor.teardown to halt events ASAP + processor.teardown end # Starts processing events and starts adapters # or resumes invoking callbacks if paused def start - transition :processing + transition :backend_started if state == :initializing + transition :frontend_ready if state == :backend_started + transition :processing_events if state == :frontend_ready + transition :processing_events if state == :paused end - # TODO: depreciate - alias_method :unpause, :start - - # Stops processing and terminates all actors + # Stops both listening for events and processing them def stop transition :stopped end @@ -126,128 +105,28 @@ def pause # processing means callbacks are called def processing? - state == :processing + state == :processing_events end def paused? state == :paused end - # TODO: deprecate - alias_method :listen?, :processing? - - # TODO: deprecate - def paused=(value) - transition value ? :paused : :processing - end - - # TODO: deprecate - alias_method :paused, :paused? - - # Add files and dirs to ignore on top of defaults - # - # (@see Listen::Silencer for default ignored files and dirs) - # def ignore(regexps) @silencer_controller.append_ignores(regexps) end - # Replace default ignore patterns with provided regexp def ignore!(regexps) @silencer_controller.replace_with_bang_ignores(regexps) end - # Listen only to files and dirs matching regexp def only(regexps) @silencer_controller.replace_with_only(regexps) end - def queue(type, change, dir, path, options = {}) - fail "Invalid type: #{type.inspect}" unless [:dir, :file].include? type - fail "Invalid change: #{change.inspect}" unless change.is_a?(Symbol) - fail "Invalid path: #{path.inspect}" unless path.is_a?(String) - if @options[:relative] - dir = begin - cwd = Pathname.pwd - dir.relative_path_from(cwd) - rescue ArgumentError - dir - end - end - event_queue << [type, change, dir, path, options] - - self.last_queue_event_time = Time.now.to_f - _wakeup_wait_thread unless state == :paused - end - - def record_for(dir) - fs_changes[dir.to_s].record - end - private - include Internals::Logging - - def _init_options(options = {}) - { - # Listener options - debug: false, - wait_for_delay: 0.1, - relative: false, - - # Backend selecting options - force_polling: false, - polling_fallback_message: nil, - - }.merge(options) - end - - def _wait_for_changes(config) - latency = options[:wait_for_delay] - EventProcessor.new(config).loop_for(latency) - rescue StandardError => ex - msg = "exception while processing events: #{ex}"\ - " Backtrace:\n -- #{ex.backtrace * "\n -- "}" - Listen::Logger.error(msg) - end - - def _silenced?(path, type) - @silencer.silenced?(path, type) - end - - attr_reader :adapter - attr_reader :queue_optimizer - attr_reader :event_queue - attr_reader :fs_changes - - attr_accessor :last_queue_event_time - - attr_reader :wait_thread - - def _start_wait_thread - config = EventProcessor::Config.new(self, event_queue, @queue_optimizer) - @wait_thread = Internals::ThreadPool.add { _wait_for_changes(config) } - end - - def _wakeup_wait_thread - wait_thread.wakeup if wait_thread && wait_thread.alive? - end - - def _stop_wait_thread - return unless wait_thread - if wait_thread.alive? - wait_thread.wakeup - wait_thread.join - end - @wait_thread = nil - end - - def _queue_raw_change(type, dir, rel_path, options) - _debug { "raw queue: #{[type, dir, rel_path, options].inspect}" } - fs_changes[dir.to_s].change(type, rel_path, options) - rescue RuntimeError - _error_exception "_queue_raw_change exception %s:\n%s:\n" - raise - end + attr_reader :processor + attr_reader :backend end end diff --git a/lib/listen/listener/config.rb b/lib/listen/listener/config.rb index ced287cd..8db77a4f 100644 --- a/lib/listen/listener/config.rb +++ b/lib/listen/listener/config.rb @@ -2,11 +2,13 @@ module Listen class Listener class Config DEFAULTS = { + # Listener options debug: false, # TODO: is this broken? - latency: nil, wait_for_delay: nil, # NOTE: should be provided by adapter if possible - force_polling: false, relative: false, + + # Backend selecting options + force_polling: false, polling_fallback_message: nil } From 39981ec21bcbdd7126c99a37d87958275ec5900b Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:43:59 +0200 Subject: [PATCH 33/47] remove TCP-related local_fs? methods --- lib/listen/queue_optimizer.rb | 18 +++--------------- spec/lib/listen/adapter/bsd_spec.rb | 1 - spec/lib/listen/adapter/darwin_spec.rb | 1 - spec/lib/listen/adapter/linux_spec.rb | 1 - spec/lib/listen/adapter/windows_spec.rb | 1 - spec/lib/listen/queue_optimizer_spec.rb | 1 - 6 files changed, 3 insertions(+), 20 deletions(-) diff --git a/lib/listen/queue_optimizer.rb b/lib/listen/queue_optimizer.rb index 9c5b6091..4fc7b890 100644 --- a/lib/listen/queue_optimizer.rb +++ b/lib/listen/queue_optimizer.rb @@ -6,10 +6,6 @@ def initialize(adapter_class, silencer) @silencer = silencer end - def local_fs? - @adapter_class.local_fs? - end - def exist?(path) Pathname(path).exist? end @@ -25,18 +21,10 @@ def debug(*args, &block) def smoosh_changes(changes) # TODO: adapter could be nil at this point (shutdown) - if config.local_fs? - cookies = changes.group_by do |_, _, _, _, options| - (options || {})[:cookie] - end - _squash_changes(_reinterpret_related_changes(cookies)) - else - smooshed = { modified: [], added: [], removed: [] } - changes.each do |_, change, dir, rel_path, _| - smooshed[change] << (dir + rel_path).to_s if smooshed.key?(change) - end - smooshed.tap { |s| s.each { |_, v| v.uniq! } } + cookies = changes.group_by do |_, _, _, _, options| + (options || {})[:cookie] end + _squash_changes(_reinterpret_related_changes(cookies)) end def initialize(config) diff --git a/spec/lib/listen/adapter/bsd_spec.rb b/spec/lib/listen/adapter/bsd_spec.rb index 82e17043..1ffe1506 100644 --- a/spec/lib/listen/adapter/bsd_spec.rb +++ b/spec/lib/listen/adapter/bsd_spec.rb @@ -1,7 +1,6 @@ RSpec.describe Listen::Adapter::BSD do describe 'class' do subject { described_class } - it { should be_local_fs } if bsd? it { should be_usable } diff --git a/spec/lib/listen/adapter/darwin_spec.rb b/spec/lib/listen/adapter/darwin_spec.rb index e87a2167..40657cb3 100644 --- a/spec/lib/listen/adapter/darwin_spec.rb +++ b/spec/lib/listen/adapter/darwin_spec.rb @@ -8,7 +8,6 @@ RSpec.describe Adapter::Darwin do describe 'class' do subject { described_class } - it { should be_local_fs } if darwin? it { should be_usable } diff --git a/spec/lib/listen/adapter/linux_spec.rb b/spec/lib/listen/adapter/linux_spec.rb index 7a316841..f80ff902 100644 --- a/spec/lib/listen/adapter/linux_spec.rb +++ b/spec/lib/listen/adapter/linux_spec.rb @@ -1,7 +1,6 @@ RSpec.describe Listen::Adapter::Linux do describe 'class' do subject { described_class } - it { should be_local_fs } if linux? it { should be_usable } diff --git a/spec/lib/listen/adapter/windows_spec.rb b/spec/lib/listen/adapter/windows_spec.rb index 94debdce..0d5216f3 100644 --- a/spec/lib/listen/adapter/windows_spec.rb +++ b/spec/lib/listen/adapter/windows_spec.rb @@ -1,7 +1,6 @@ RSpec.describe Listen::Adapter::Windows do describe 'class' do subject { described_class } - it { should be_local_fs } if windows? it { should be_usable } diff --git a/spec/lib/listen/queue_optimizer_spec.rb b/spec/lib/listen/queue_optimizer_spec.rb index 93494273..eab23602 100644 --- a/spec/lib/listen/queue_optimizer_spec.rb +++ b/spec/lib/listen/queue_optimizer_spec.rb @@ -3,7 +3,6 @@ subject { described_class.new(config) } before do - allow(config).to receive(:local_fs?).and_return(true) allow(config).to receive(:debug) end From a7b9f672a590bd34b2f1525912a531d27ec39294 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:46:59 +0200 Subject: [PATCH 34/47] allow choosing poling/native acceptance tests --- spec/acceptance/listen_spec.rb | 14 +++++++++++++- 1 file changed, 13 insertions(+), 1 deletion(-) diff --git a/spec/acceptance/listen_spec.rb b/spec/acceptance/listen_spec.rb index 70cec336..3eaad602 100644 --- a/spec/acceptance/listen_spec.rb +++ b/spec/acceptance/listen_spec.rb @@ -27,7 +27,19 @@ end end - [false, true].each do |polling| + modes = + case ENV['TEST_LISTEN_ADAPTER_MODES'] || 'both' + when 'polling' + [true] + when 'native' + [false] + else + [false, true] + end + + # TODO: make it configurable + # TODO: restore + modes.each do |polling| context "force_polling option to #{polling}" do let(:polling_options) { { force_polling: polling } } From 80a57f90024876083555dd4a8fd92b2d5ca22e68 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:47:41 +0200 Subject: [PATCH 35/47] rework Adapter spec to test Record usage --- spec/lib/listen/adapter/base_spec.rb | 97 +++++++++++++++++++++++----- 1 file changed, 82 insertions(+), 15 deletions(-) diff --git a/spec/lib/listen/adapter/base_spec.rb b/spec/lib/listen/adapter/base_spec.rb index 27fff329..7d0741f3 100644 --- a/spec/lib/listen/adapter/base_spec.rb +++ b/spec/lib/listen/adapter/base_spec.rb @@ -1,28 +1,95 @@ -include Listen - -RSpec.describe Adapter::Base do +RSpec.describe Listen::Adapter::Base do class FakeAdapter < described_class - def initialize(*args) - super(*args) + def initialize(config) + @my_callbacks = {} + super + end + + def _run + fail NotImplementedError + end + + def _configure(dir, &callback) + @my_callbacks[dir.to_s] = callback + end + + def fake_event(event) + dir = event[:dir] + @my_callbacks[dir].call(event) + end + + def _process_event(dir, event) + _queue_change(:file, dir, event[:file], cookie: event[:cookie]) end end - subject { FakeAdapter.new(mq: mq, directories: []) } + let(:dir1) { instance_double(Pathname, 'dir1', to_s: '/foo/dir1') } + + let(:config) { instance_double(Listen::Adapter::Config) } + let(:queue) { instance_double(Queue) } + let(:silencer) { instance_double(Listen::Silencer) } + let(:adapter_options) { {} } + + let(:snapshot) { instance_double(Listen::Change) } + let(:record) { instance_double(Listen::Record) } + + subject { FakeAdapter.new(config) } + + before do + allow(config).to receive(:directories).and_return([dir1]) + allow(config).to receive(:queue).and_return(queue) + allow(config).to receive(:silencer).and_return(silencer) + allow(config).to receive(:adapter_options).and_return(adapter_options) + + allow(Listen::Internals::ThreadPool). + to receive(:add) { |&block| block.call } - let(:mq) { instance_double(Listener) } + # Stuff that happens in configure() + allow(Listen::Record).to receive(:new).with(dir1).and_return(record) - describe '#_notify_change' do - let(:dir) { Pathname.pwd } + allow(Listen::Change::Config).to receive(:new).with(queue, silencer). + and_return(config) + + allow(Listen::Change).to receive(:new).with(config, record). + and_return(snapshot) + end + + describe '#start' do + before do + allow(subject).to receive(:_run) + + allow(snapshot).to receive(:record).and_return(record) + allow(record).to receive(:build) + end + + it 'builds record' do + expect(record).to receive(:build) + subject.start + end + + it 'runs the adapter' do + expect(subject).to receive(:_run) + subject.start + end + end + + describe 'handling events' do + before do + allow(subject).to receive(:_run) + + allow(snapshot).to receive(:record).and_return(record) + allow(record).to receive(:build) + end - context 'listener is listening or paused' do - let(:worker) { instance_double(Change) } + context 'when an event occurs' do + it 'passes invalidates the snapshot based on the event' do + subject.start - it 'calls change on change_pool asynchronously' do - expect(mq).to receive(:_queue_raw_change). - with(:dir, dir, 'path', recursive: true) + expect(snapshot).to receive(:invalidate).with(:file, 'bar', cookie: 3) - subject.send(:_queue_change, :dir, dir, 'path', recursive: true) + event = { dir: '/foo/dir1', file: 'bar', type: :moved, cookie: 3 } + subject.fake_event(event) end end end From 52a54ff3be5dc29e9d257d84c2d92f715a8acc86 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:48:29 +0200 Subject: [PATCH 36/47] fix/improve Darwin specs for Record usage --- spec/lib/listen/adapter/darwin_spec.rb | 78 +++++++++++++++++--------- 1 file changed, 51 insertions(+), 27 deletions(-) diff --git a/spec/lib/listen/adapter/darwin_spec.rb b/spec/lib/listen/adapter/darwin_spec.rb index 40657cb3..6bbc4ce0 100644 --- a/spec/lib/listen/adapter/darwin_spec.rb +++ b/spec/lib/listen/adapter/darwin_spec.rb @@ -17,13 +17,22 @@ end let(:options) { {} } - let(:mq) { instance_double(Listener, options: options) } + let(:config) { instance_double(Listen::Adapter::Config) } + let(:queue) { instance_double(Queue) } + let(:silencer) { instance_double(Listen::Silencer) } + + let(:dir1) { fake_path('/foo/dir1', cleanpath: fake_path('/foo/dir1')) } + let(:directories) { [dir1] } + + subject { described_class.new(config) } + + before do + allow(config).to receive(:directories).and_return(directories) + allow(config).to receive(:adapter_options).and_return(options) + end describe '#_latency' do - subject do - adapter = described_class.new(options.merge(mq: mq, directories: [])) - adapter.options.latency - end + subject { described_class.new(config).options.latency } context 'with no overriding option' do it { should eq 0.1 } @@ -36,18 +45,22 @@ end describe 'multiple dirs' do - subject do - dirs = config.keys.map { |p| Pathname(p.to_s) } - described_class.new(directories: dirs) + let(:dir1) { fake_path('/foo/dir1', cleanpath: fake_path('/foo/dir1')) } + let(:dir2) { fake_path('/foo/dir2', cleanpath: fake_path('/foo/dir1')) } + let(:dir3) { fake_path('/foo/dir3', cleanpath: fake_path('/foo/dir1')) } + + before do + allow(config).to receive(:queue).and_return(queue) + allow(config).to receive(:silencer).and_return(silencer) end - let(:foo1) { double('foo1') } - let(:foo2) { double('foo2') } - let(:foo3) { double('foo3') } + let(:foo1) { double('fsevent1') } + let(:foo2) { double('fsevent2') } + let(:foo3) { double('fsevent3') } before do - allow(FSEvent).to receive(:new).and_return(*config.values, nil) - config.each do |dir, obj| + allow(FSEvent).to receive(:new).and_return(*expectations.values, nil) + expectations.each do |dir, obj| allow(obj).to receive(:watch).with(dir.to_s, latency: 0.1) end subject.configure @@ -55,15 +68,18 @@ describe 'configuration' do context 'with 1 directory' do - let(:config) { { dir1: foo1 } } + let(:directories) { expectations.keys.map { |p| Pathname(p.to_s) } } + + let(:expectations) { { '/foo/dir1': foo1 } } it 'configures directory' do - expect(foo1).to have_received(:watch).with('dir1', latency: 0.1) + expect(foo1).to have_received(:watch).with('/foo/dir1', latency: 0.1) end end context 'with 2 directories' do - let(:config) { { dir1: foo1, dir2: foo2 } } + let(:directories) { expectations.keys.map { |p| Pathname(p.to_s) } } + let(:expectations) { { dir1: foo1, dir2: foo2 } } it 'configures directories' do expect(foo1).to have_received(:watch).with('dir1', latency: 0.1) @@ -72,18 +88,26 @@ end context 'with 3 directories' do - let(:config) { { dir1: foo1, dir2: foo2, dir3: foo3 } } + let(:directories) { expectations.keys.map { |p| Pathname(p.to_s) } } + let(:expectations) do + { + '/foo/dir1': foo1, + '/foo/dir2': foo2, + '/foo/dir3': foo3 + } + end it 'configures directories' do - expect(foo1).to have_received(:watch).with('dir1', latency: 0.1) - expect(foo2).to have_received(:watch).with('dir2', latency: 0.1) - expect(foo3).to have_received(:watch).with('dir3', latency: 0.1) + expect(foo1).to have_received(:watch).with('/foo/dir1', latency: 0.1) + expect(foo2).to have_received(:watch).with('/foo/dir2', latency: 0.1) + expect(foo3).to have_received(:watch).with('/foo/dir3', latency: 0.1) end end end describe 'running threads' do let(:running) { [] } + let(:directories) { expectations.keys.map { |p| Pathname(p.to_s) } } before do started = Queue.new @@ -95,7 +119,7 @@ max_test_time = 3 * thread_start_overhead block_time = max_test_time + thread_start_overhead - config.each do |name, obj| + expectations.each do |name, obj| left << name # anything, we're just counting allow(obj).to receive(:run).once do threads << Thread.current @@ -118,23 +142,23 @@ end context 'with 1 directory' do - let(:config) { { dir1: foo1 } } + let(:expectations) { { dir1: foo1 } } it 'runs all the workers without blocking' do - expect(running.sort).to eq(config.keys) + expect(running.sort).to eq(expectations.keys) end end context 'with 2 directories' do - let(:config) { { dir1: foo1, dir2: foo2 } } + let(:expectations) { { dir1: foo1, dir2: foo2 } } it 'runs all the workers without blocking' do - expect(running.sort).to eq(config.keys) + expect(running.sort).to eq(expectations.keys) end end context 'with 3 directories' do - let(:config) { { dir1: foo1, dir2: foo2, dir3: foo3 } } + let(:expectations) { { dir1: foo1, dir2: foo2, dir3: foo3 } } it 'runs all the workers without blocking' do - expect(running.sort).to eq(config.keys) + expect(running.sort).to eq(expectations.keys) end end end From ca19d3d54af0dd8dcf64538b824527b1ce121aa7 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:48:56 +0200 Subject: [PATCH 37/47] fix and improve Linux adapter for Record usage --- spec/lib/listen/adapter/linux_spec.rb | 70 +++++++++++++++++----- spec/lib/listen/adapter/polling_spec.rb | 77 +++++++++++++++++-------- 2 files changed, 111 insertions(+), 36 deletions(-) diff --git a/spec/lib/listen/adapter/linux_spec.rb b/spec/lib/listen/adapter/linux_spec.rb index f80ff902..ac04e057 100644 --- a/spec/lib/listen/adapter/linux_spec.rb +++ b/spec/lib/listen/adapter/linux_spec.rb @@ -10,13 +10,30 @@ end if linux? - let(:directories) { [] } - let(:mq) { instance_double(Listen::Listener) } + let(:dir1) do + instance_double( + Pathname, + 'dir1', + to_s: '/foo/dir1', + cleanpath: real_dir1 + ) + end + + # just so cleanpath works in above double + let(:real_dir1) { instance_double(Pathname, 'dir1', to_s: '/foo/dir1') } + + let(:config) { instance_double(Listen::Adapter::Config) } + let(:queue) { instance_double(Queue) } + let(:silencer) { instance_double(Listen::Silencer) } + let(:snapshot) { instance_double(Listen::Change) } + let(:record) { instance_double(Listen::Record) } - subject { described_class.new(mq: mq, directories: directories) } + # TODO: fix other adapters too! + subject { described_class.new(config) } describe 'inotify limit message' do let(:directories) { [Pathname.pwd] } + let(:adapter_options) { {} } before do require 'rb-inotify' @@ -25,6 +42,9 @@ fake_notifier = double(:fake_notifier, new: fake_worker) stub_const('INotify::Notifier', fake_notifier) + + allow(config).to receive(:directories).and_return(directories) + allow(config).to receive(:adapter_options).and_return(adapter_options) end it 'should be shown before calling abort' do @@ -33,18 +53,42 @@ end end + # TODO: should probably be adapted to be more like adapter/base_spec.rb describe '_callback' do - let(:directories) { [Pathname.pwd] } - before { subject.configure } + let(:directories) { [dir1] } + let(:adapter_options) { { events: [:recursive, :close_write] } } + + before do + allow(Kernel).to receive(:require).with('rb-inotify') + fake_worker = double(:fake_worker) + events = [:recursive, :close_write] + allow(fake_worker).to receive(:watch).with('/foo/dir1', *events) + + fake_notifier = double(:fake_notifier, new: fake_worker) + stub_const('INotify::Notifier', fake_notifier) + + allow(config).to receive(:directories).and_return(directories) + allow(config).to receive(:adapter_options).and_return(adapter_options) + allow(config).to receive(:queue).and_return(queue) + allow(config).to receive(:silencer).and_return(silencer) + + allow(Listen::Record).to receive(:new).with(dir1).and_return(record) + allow(Listen::Change::Config).to receive(:new).with(queue, silencer). + and_return(config) + allow(Listen::Change).to receive(:new).with(config, record). + and_return(snapshot) + + subject.configure + end + let(:expect_change) do lambda do |change| - allow(mq).to receive(:_queue_raw_change). - with( - :file, - Pathname.pwd, - 'path/foo.txt', - change: change, - cookie: 123) + expect(snapshot).to receive(:invalidate).with( + :file, + 'path/foo.txt', + cookie: 123, + change: change + ) end end @@ -55,7 +99,7 @@ callback.call double( :inotify_event, name: 'foo.txt', - watcher: double(:watcher, path: (Pathname.pwd + 'path').to_s), + watcher: double(:watcher, path: '/foo/dir1/path'), flags: flags, cookie: 123) end diff --git a/spec/lib/listen/adapter/polling_spec.rb b/spec/lib/listen/adapter/polling_spec.rb index 7266d09f..7a4ce4a5 100644 --- a/spec/lib/listen/adapter/polling_spec.rb +++ b/spec/lib/listen/adapter/polling_spec.rb @@ -3,44 +3,75 @@ RSpec.describe Adapter::Polling do describe 'class' do subject { described_class } - it { should be_local_fs } it { should be_usable } end subject do - described_class.new(options.merge(mq: mq, directories: directories)) + described_class.new(config) end + let(:dir1) do + instance_double(Pathname, 'dir1', to_s: '/foo/dir1', cleanpath: real_dir1) + end + + # just so cleanpath works in above double + let(:real_dir1) { instance_double(Pathname, 'dir1', to_s: '/foo/dir1') } + + let(:config) { instance_double(Listen::Adapter::Config) } + let(:directories) { [dir1] } let(:options) { {} } - let(:mq) { instance_double(Listener, options: options) } + let(:queue) { instance_double(Queue) } + let(:silencer) { instance_double(Listen::Silencer) } + let(:snapshot) { instance_double(Listen::Change) } - describe '#start' do - let(:directories) { [Pathname.pwd] } + let(:record) { instance_double(Listen::Record) } - it 'notifies change on every listener directories path' do - expect(mq).to receive(:_queue_raw_change). - with(:dir, Pathname.pwd, '.', recursive: true) + context 'with a valid configuration' do + before do + allow(config).to receive(:directories).and_return(directories) + allow(config).to receive(:adapter_options).and_return(options) + allow(config).to receive(:queue).and_return(queue) + allow(config).to receive(:silencer).and_return(silencer) - t = Thread.new { subject.start } - sleep 0.25 - t.kill - t.join - end - end + allow(Listen::Record).to receive(:new).with(dir1).and_return(record) - describe '#_latency' do - subject do - adapter = described_class.new(options.merge(mq: mq, directories: [])) - adapter.options.latency + allow(Listen::Change).to receive(:new).with(config, record). + and_return(snapshot) + allow(Listen::Change::Config).to receive(:new).with(queue, silencer). + and_return(config) end - context 'with no overriding option' do - it { should eq 1.0 } + describe '#start' do + before do + allow(snapshot).to receive(:record).and_return(record) + allow(record).to receive(:build) + end + + it 'notifies change on every listener directories path' do + expect(snapshot).to receive(:invalidate). + with(:dir, '.', recursive: true) + + t = Thread.new { subject.start } + sleep 0.25 + t.kill + t.join + end end - context 'with custom latency overriding' do - let(:options) { { latency: 1234 } } - it { should eq 1234 } + describe '#_latency' do + subject do + adapter = described_class.new(config) + adapter.options.latency + end + + context 'with no overriding option' do + it { should eq 1.0 } + end + + context 'with custom latency overriding' do + let(:options) { { latency: 1234 } } + it { should eq 1234 } + end end end end From f738db74ff273c7918074204ccc6eabf6230834e Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:51:55 +0200 Subject: [PATCH 38/47] refactor and update Directory spec --- spec/lib/listen/directory_spec.rb | 118 +++++++++++++++++++++++------- 1 file changed, 90 insertions(+), 28 deletions(-) diff --git a/spec/lib/listen/directory_spec.rb b/spec/lib/listen/directory_spec.rb index e9e97e85..5a9f3968 100644 --- a/spec/lib/listen/directory_spec.rb +++ b/spec/lib/listen/directory_spec.rb @@ -1,10 +1,20 @@ include Listen RSpec.describe Directory do + def fake_file_stat(name, options = {}) + defaults = { directory?: false } + instance_double(::File::Stat, name, defaults.merge(options)) + end + + def fake_dir_stat(name, options = {}) + defaults = { directory?: true } + instance_double(::File::Stat, name, defaults.merge(options)) + end + let(:dir) { double(:dir) } - let(:file) { double(:file, directory?: false, to_s: 'file.rb') } - let(:file2) { double(:file2, directory?: false, to_s: 'file2.rb') } - let(:subdir) { double(:subdir, directory?: true, to_s: 'subdir') } + let(:file) { fake_path('file.rb') } + let(:file2) { fake_path('file2.rb') } + let(:subdir) { fake_path('subdir') } let(:record) do instance_double( @@ -15,7 +25,7 @@ unset_path: true) end - let(:snapshot) { instance_double(Change, change: nil, record: record) } + let(:snapshot) { instance_double(Change, record: record, invalidate: nil) } before do allow(dir).to receive(:+).with('.') { dir } @@ -28,6 +38,10 @@ allow(Pathname).to receive(:new).with('some_dir').and_return(dir) allow(Pathname).to receive(:new).with('.').and_return(dir) + + allow(::File).to receive(:lstat) do |*args| + fail "Not stubbed: File.lstat(#{args.map(&:inspect) * ','})" + end end context '#scan with recursive off' do @@ -47,28 +61,64 @@ end it "snapshots changes for file path and dir that doesn't exist" do - expect(snapshot).to receive(:change).with(:file, 'file.rb', {}) + expect(snapshot).to receive(:invalidate).with(:file, 'file.rb', {}) - expect(snapshot).to receive(:change). + expect(snapshot).to receive(:invalidate). with(:dir, 'subdir', recursive: false) described_class.scan(snapshot, '.', options) end end - context 'with only file2.rb in dir' do - before { allow(dir).to receive(:children) { [file2] } } + context 'when subdir is removed' do + before do + allow(dir).to receive(:children) { [file] } - it 'notices file & file2 and no longer existing dir' do - expect(snapshot).to receive(:change).with(:file, 'file.rb', {}) - expect(snapshot).to receive(:change).with(:file, 'file2.rb', {}) + allow(::File).to receive(:lstat).with('file.rb'). + and_return(fake_file_stat('file.rb')) + end - expect(snapshot).to receive(:change). + it 'notices subdir does not exist' do + expect(snapshot).to receive(:invalidate). with(:dir, 'subdir', recursive: false) described_class.scan(snapshot, '.', options) end end + + context 'when file.rb removed' do + before do + allow(dir).to receive(:children) { [subdir] } + + allow(::File).to receive(:lstat).with('subdir'). + and_return(fake_dir_stat('subdir')) + end + + it 'notices file was removed' do + expect(snapshot).to receive(:invalidate).with(:file, 'file.rb', {}) + described_class.scan(snapshot, '.', options) + end + end + + context 'when file2.rb is added' do + before do + allow(dir).to receive(:children) { [file, file2, subdir] } + + allow(::File).to receive(:lstat).with('file.rb'). + and_return(fake_file_stat('file.rb')) + + allow(::File).to receive(:lstat).with('file2.rb'). + and_return(fake_file_stat('file2.rb')) + + allow(::File).to receive(:lstat).with('subdir'). + and_return(fake_dir_stat('subdir')) + end + + it 'notices file removed and file2 changed' do + expect(snapshot).to receive(:invalidate).with(:file, 'file2.rb', {}) + described_class.scan(snapshot, '.', options) + end + end end context 'with empty record' do @@ -78,7 +128,7 @@ before { allow(dir).to receive(:children) { fail Errno::ENOENT } } it 'reports no changes' do - expect(snapshot).to_not receive(:change) + expect(snapshot).to_not receive(:invalidate) described_class.scan(snapshot, '.', options) end @@ -92,7 +142,7 @@ before { allow(dir).to receive(:children) { fail Errno::EHOSTDOWN } } it 'reports no changes' do - expect(snapshot).to_not receive(:change) + expect(snapshot).to_not receive(:invalidate) described_class.scan(snapshot, '.', options) end @@ -103,13 +153,21 @@ end context 'with file.rb in dir' do - before { allow(dir).to receive(:children) { [file] } } + before do + allow(dir).to receive(:children) { [file] } + + allow(::File).to receive(:lstat).with('file.rb'). + and_return(fake_file_stat('file.rb')) + end it 'snapshots changes for file & file2 paths' do - expect(snapshot).to receive(:change).with(:file, 'file.rb', {}) - expect(snapshot).to_not receive(:change).with(:file, 'file2.rb', {}) + expect(snapshot).to receive(:invalidate). + with(:file, 'file.rb', {}) + + expect(snapshot).to_not receive(:invalidate). + with(:file, 'file2.rb', {}) - expect(snapshot).to_not receive(:change). + expect(snapshot).to_not receive(:invalidate). with(:dir, 'subdir', recursive: false) described_class.scan(snapshot, '.', options) @@ -132,30 +190,33 @@ end it 'snapshots changes for file & subdir path' do - expect(snapshot).to receive(:change).with(:file, 'file.rb', {}) + expect(snapshot).to receive(:invalidate).with(:file, 'file.rb', {}) - expect(snapshot).to receive(:change). + expect(snapshot).to receive(:invalidate). with(:dir, 'subdir', recursive: true) described_class.scan(snapshot, '.', options) end end - context 'with subdir2 path present in dir' do - let(:subdir2) { double(:subdir2, directory?: true, children: []) } + context 'with subdir2 path present' do + let(:subdir2) { fake_path('subdir2', children: []) } before do allow(dir).to receive(:children) { [subdir2] } allow(subdir2).to receive(:relative_path_from).with(dir) { 'subdir2' } + + allow(::File).to receive(:lstat).with('subdir2'). + and_return(fake_dir_stat('subdir2')) end it 'snapshots changes for file, file2 & subdir paths' do - expect(snapshot).to receive(:change).with(:file, 'file.rb', {}) + expect(snapshot).to receive(:invalidate).with(:file, 'file.rb', {}) - expect(snapshot).to receive(:change). + expect(snapshot).to receive(:invalidate). with(:dir, 'subdir', recursive: true) - expect(snapshot).to receive(:change). + expect(snapshot).to receive(:invalidate). with(:dir, 'subdir2', recursive: true) described_class.scan(snapshot, '.', options) @@ -172,20 +233,21 @@ end it 'reports no changes' do - expect(snapshot).to_not receive(:change) + expect(snapshot).to_not receive(:invalidate) described_class.scan(snapshot, '.', options) end end context 'with subdir present in dir' do - before do allow(dir).to receive(:children) { [subdir] } allow(subdir).to receive(:children) { [] } + allow(::File).to receive(:lstat).with('subdir'). + and_return(fake_dir_stat('subdir')) end it 'snapshots changes for subdir' do - expect(snapshot).to receive(:change). + expect(snapshot).to receive(:invalidate). with(:dir, 'subdir', recursive: true) described_class.scan(snapshot, '.', options) From 76c3b64ba82b39018e5b12e230f7ece108b62cd0 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:54:40 +0200 Subject: [PATCH 39/47] rework and update Listener spec --- spec/lib/listen/listener_spec.rb | 345 ++++++++++--------------------- 1 file changed, 106 insertions(+), 239 deletions(-) diff --git a/spec/lib/listen/listener_spec.rb b/spec/lib/listen/listener_spec.rb index 040cbdeb..ac603583 100644 --- a/spec/lib/listen/listener_spec.rb +++ b/spec/lib/listen/listener_spec.rb @@ -2,318 +2,208 @@ RSpec.describe Listener do - let(:realdir1) { instance_double(Pathname, '/foo/dir1', children: []) } - let(:realdir2) { instance_double(Pathname, '/foo/dir2', children: []) } + let(:realdir1) { fake_path('/foo/dir1', children: []) } + let(:realdir2) { fake_path('/foo/dir2', children: []) } - let(:queue) { instance_double(Queue) } - - let(:dir1) { instance_double(Pathname, 'dir1', realpath: realdir1) } - let(:dir2) { instance_double(Pathname, 'dir2', realpath: realdir2) } + let(:dir1) { fake_path('dir1', realpath: realdir1) } + let(:dir2) { fake_path('dir2', realpath: realdir2) } let(:dirs) { ['dir1'] } - subject { described_class.new(*(dirs + [options]).compact) } + let(:block) { instance_double(Proc) } + + subject do + described_class.new(*(dirs + [options]).compact) do |*changes| + block.call(*changes) + end + end let(:options) { {} } let(:record) { instance_double(Record, build: true, root: 'dir2') } let(:silencer) { instance_double(Silencer, configure: nil) } - let(:adapter_namespace) do - class_double('Listen::Adapter'). - as_stubbed_const(transfer_nested_constants: true) - end + let(:backend_class) { class_double('Listen::Backend') } - let(:adapter_class) { class_double('Listen::Adapter::Base') } - let(:adapter) { instance_double('Listen::Adapter::Base', start: nil) } + let(:backend) { instance_double(Backend) } let(:optimizer_config) { instance_double(QueueOptimizer::Config) } let(:optimizer) { instance_double(QueueOptimizer) } - let(:processor_config) { instance_double(EventProcessor::Config) } - let(:processor) { instance_double(EventProcessor) } + let(:processor_config) { instance_double(Event::Config) } + let(:processor) { instance_double(Event::Loop) } + + let(:event_queue) { instance_double(Event::Queue) } let(:default_latency) { 0.1 } + let(:backend_wait_for_delay) { 0.123 } + + let(:processing_thread) { instance_double(Thread) } before do allow(Silencer).to receive(:new) { silencer } - # TODO: use a configuration object to clean this up - allow(adapter_namespace).to receive(:select). - with(anything).and_return(adapter_class) + allow(Backend).to receive(:new). + with(anything, event_queue, silencer, anything). + and_return(backend) + + allow(backend).to receive(:min_delay_between_events). + and_return(backend_wait_for_delay) - allow(adapter_class).to receive(:new).with(anything).and_return(adapter) - allow(adapter_class).to receive(:local_fs?).and_return(true) - allow(adapter).to receive(:class).and_return(adapter_class) + # TODO: use a configuration object to clean this up - allow(QueueOptimizer::Config).to receive(:new). - with(adapter_class, silencer).and_return(optimizer_config) + allow(QueueOptimizer::Config).to receive(:new).with(backend, silencer). + and_return(optimizer_config) allow(QueueOptimizer).to receive(:new).with(optimizer_config). and_return(optimizer) - allow(EventProcessor::Config).to receive(:new). - with(anything, queue, optimizer).and_return(processor_config) + allow(Event::Queue).to receive(:new).and_return(event_queue) - allow(EventProcessor).to receive(:new).with(processor_config). - and_return(processor) + allow(Event::Config).to receive(:new). + with(anything, event_queue, optimizer, backend_wait_for_delay). + and_return(processor_config) - allow(processor).to receive(:loop_for).with(default_latency) + allow(Event::Loop).to receive(:new).with(processor_config). + and_return(processor) allow(Record).to receive(:new).and_return(record) allow(Pathname).to receive(:new).with('dir1').and_return(dir1) allow(Pathname).to receive(:new).with('dir2').and_return(dir2) - allow(Queue).to receive(:new).and_return(queue) - allow(queue).to receive(:<<) - allow(queue).to receive(:empty?).and_return(true) + allow(Internals::ThreadPool).to receive(:add).and_return(processing_thread) + allow(processing_thread).to receive(:alive?).and_return(true) + allow(processing_thread).to receive(:wakeup) + allow(processing_thread).to receive(:join) - allow(Internals::ThreadPool).to receive(:add) + allow(block).to receive(:call) end describe 'initialize' do it { should_not be_paused } context 'with a block' do - describe 'block' do - subject { described_class.new('dir1', &(proc {})) } - specify { expect(subject.block).to_not be_nil } + let(:myblock) { instance_double(Proc) } + let(:block) { proc { myblock.call } } + subject { described_class.new('dir1', &block) } + + it 'passes the block to the event processor' do + allow(Event::Config).to receive(:new) do |*_args, &some_block| + expect(some_block).to be + some_block.call + processor_config + end + expect(myblock).to receive(:call) + subject end end context 'with directories' do - describe 'directories' do - subject { described_class.new('dir1', 'dir2') } - specify { expect(subject.directories).to eq([realdir1, realdir2]) } - end - end - end - - describe 'options' do - context 'with supported adapter option' do - let(:options) { { latency: 1.234 } } - before do - allow(supervisor).to receive(:add) - allow(Adapter).to receive(:select) { Adapter::Polling } - end - - it 'passes adapter options to adapter' do - expect(supervisor).to receive(:add). - with(anything, hash_including( - args: [hash_including(latency: 1.234)] - )) - subject.start - end - end + subject { described_class.new('dir1', 'dir2') } - context 'with unsupported adapter option' do - let(:options) { { latency: 1.234 } } - before do - allow(supervisor).to receive(:add) - allow(Adapter).to receive(:select) { Adapter::Linux } - end - - it 'passes adapter options to adapter' do - expect(supervisor).to_not receive(:add). - with(anything, hash_including( - args: [hash_including(latency: anything)] - )) - subject.start - end - end - - context 'default options' do - it 'sets default options' do - expect(subject.options). - to eq( - debug: false, - wait_for_delay: 0.1, - force_polling: false, - relative: false, - polling_fallback_message: nil) - end - end - - context 'custom options' do - subject do - described_class.new( - 'dir1', - latency: 1.234, - wait_for_delay: 0.85, - relative: true) - end - - it 'sets new options on initialize' do - expect(subject.options). - to eq( - debug: false, - latency: 1.234, - wait_for_delay: 0.85, - force_polling: false, - relative: true, - polling_fallback_message: nil) + it 'passes directories to backend' do + allow(Backend).to receive(:new). + with(['dir1', 'dir2'], anything, anything, anything). + and_return(backend) + subject end end end describe '#start' do before do - allow(adapter).to receive(:start) + allow(backend).to receive(:start) allow(silencer).to receive(:silenced?) { false } end - it 'builds record' do - expect(record).to receive(:build) - subject.start - end - it 'sets paused to false' do + allow(processor).to receive(:setup) + allow(processor).to receive(:resume) subject.start expect(subject).to_not be_paused end it 'starts adapter' do - expect(adapter).to receive(:start) + expect(backend).to receive(:start) + allow(processor).to receive(:setup) + allow(processor).to receive(:resume) subject.start end - - context 'when relative option is true' do - before do - current_path = instance_double(Pathname, to_s: '/project/path') - allow(Pathname).to receive(:new).with(Dir.pwd).and_return(current_path) - end - - context 'when watched dir is the current dir' do - let(:options) { { relative: true, directories: Pathname.pwd } } - it 'registers relative paths' do - event_dir = instance_double(Pathname) - dir_rel_path = instance_double(Pathname, to_s: '.') - foo_rel_path = instance_double(Pathname, to_s: 'foo', exist?: true) - - allow(event_dir).to receive(:relative_path_from). - with(Pathname.pwd). - and_return(dir_rel_path) - - allow(dir_rel_path).to receive(:+).with('foo') { foo_rel_path } - - block_stub = instance_double(Proc) - expect(block_stub).to receive(:call).with(['foo'], [], []) - subject.block = block_stub - - subject.start - subject.queue(:file, :modified, event_dir, 'foo') - subject.block.call(['foo'], [], []) - sleep 0.25 - end - end - - context 'when watched dir is not the current dir' do - let(:options) { { relative: true } } - - it 'registers relative path' do - event_dir = instance_double(Pathname) - dir_rel_path = instance_double(Pathname, to_s: '..') - foo_rel_path = instance_double(Pathname, to_s: '../foo', exist?: true) - - allow(event_dir).to receive(:relative_path_from). - with(Pathname.pwd). - and_return(dir_rel_path) - - allow(dir_rel_path).to receive(:+).with('foo') { foo_rel_path } - - block_stub = instance_double(Proc) - expect(block_stub).to receive(:call).with(['../foo'], [], []) - subject.block = block_stub - - subject.start - subject.queue(:file, :modified, event_dir, 'foo') - subject.block.call(['../foo'], [], []) - end - end - - context 'when watched dir is on another drive' do - let(:options) { { relative: true } } - - it 'registers full path' do - event_dir = instance_double(Pathname, 'event_dir', realpath: 'd:/foo') - - foo_rel_path = instance_double( - Pathname, - 'rel_path', - to_s: 'd:/foo', - exist?: true, - children: [] - ) - - allow(event_dir).to receive(:relative_path_from). - with(Pathname.pwd). - and_raise(ArgumentError) - - allow(event_dir).to receive(:+).with('foo') { foo_rel_path } - - block_stub = instance_double(Proc) - expect(block_stub).to receive(:call).with(['d:/foo'], [], []) - subject.block = block_stub - - subject.start - subject.queue(:file, :modified, event_dir, 'foo') - subject.block.call(['d:/foo'], [], []) - end - end - - end end describe '#stop' do before do + allow(backend).to receive(:start) + allow(processor).to receive(:setup) + allow(processor).to receive(:resume) subject.start end it 'terminates' do + allow(backend).to receive(:stop) + allow(processor).to receive(:teardown) subject.stop end end describe '#pause' do - before { subject.start } + before do + allow(backend).to receive(:start) + allow(processor).to receive(:setup) + allow(processor).to receive(:resume) + subject.start + end it 'sets paused to true' do + allow(processor).to receive(:pause) subject.pause expect(subject).to be_paused end end - describe '#unpause' do + describe 'unpause with start' do before do + allow(backend).to receive(:start) + allow(processor).to receive(:setup) + allow(processor).to receive(:resume) subject.start + allow(processor).to receive(:pause) subject.pause end it 'sets paused to false' do - subject.unpause + subject.start expect(subject).to_not be_paused end end describe '#paused?' do - before { subject.start } + before do + allow(backend).to receive(:start) + allow(processor).to receive(:setup) + allow(processor).to receive(:resume) + subject.start + end + it 'returns true when paused' do - subject.paused = true + allow(processor).to receive(:pause) + subject.pause expect(subject).to be_paused end - it 'returns false when not paused (nil)' do - subject.paused = nil - expect(subject).not_to be_paused - end - it 'returns false when not paused (false)' do - subject.paused = false + + it 'returns false when not paused' do expect(subject).not_to be_paused end end describe '#listen?' do context 'when processing' do - before { subject.start } + before do + allow(backend).to receive(:start) + allow(processor).to receive(:setup) + allow(processor).to receive(:resume) + subject.start + end it { should be_processing } end @@ -323,9 +213,14 @@ context 'when paused' do before do + allow(backend).to receive(:start) + allow(processor).to receive(:setup) + allow(processor).to receive(:resume) subject.start + allow(processor).to receive(:pause) subject.pause end + it { should_not be_processing } end end @@ -411,23 +306,8 @@ end describe 'processing changes' do - # TODO: this is an event processer test - it 'gets two changes and calls the block once' do - allow(silencer).to receive(:silenced?) { false } - - subject.block = proc do |modified, added, _| - expect(modified).to eql(['foo/bar.txt']) - expect(added).to eql(['foo/baz.txt']) - end - - dir = instance_double(Pathname, children: %w(bar.txt baz.txt)) - - allow(queue).to receive(:<<) - - subject.start - subject.queue(:file, :modified, dir, 'bar.txt', {}) - subject.queue(:file, :added, dir, 'baz.txt', {}) - subject.block.call(['foo/bar.txt'], ['foo/baz.txt'], []) + before do + allow(backend).to receive(:start) end end @@ -436,18 +316,5 @@ subject.stop allow(silencer).to receive(:silenced?) { true } end - - it 'queuing does not crash when changes come in' do - expect do - # TODO: write directly to queue - subject.send( - :_queue_raw_change, - :dir, - realdir1, - 'path', - recursive: true) - - end.to_not raise_error - end end end From 3f345794c83329ce1dcc4cfbca4ebd3005c7a3ea Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:55:04 +0200 Subject: [PATCH 40/47] completely rework QueueOptimizer spec --- spec/lib/listen/queue_optimizer_spec.rb | 232 ++++++++---------------- 1 file changed, 78 insertions(+), 154 deletions(-) diff --git a/spec/lib/listen/queue_optimizer_spec.rb b/spec/lib/listen/queue_optimizer_spec.rb index eab23602..0f979883 100644 --- a/spec/lib/listen/queue_optimizer_spec.rb +++ b/spec/lib/listen/queue_optimizer_spec.rb @@ -2,183 +2,107 @@ let(:config) { instance_double(Listen::QueueOptimizer::Config) } subject { described_class.new(config) } + # watched dir + let(:dir) { fake_path('dir') } + + # files + let(:foo) { fake_path('foo') } + let(:bar) { fake_path('bar') } + let(:ignored) { fake_path('ignored') } + before do allow(config).to receive(:debug) - end - - describe '_smoosh_changes' do - it 'recognizes rename from temp file' do - bar = instance_double( - Pathname, - to_s: 'bar', - exist?: true, - directory?: false) - - foo = instance_double(Pathname, to_s: 'foo', children: []) - allow(foo).to receive(:+).with('bar') { bar } - allow(config).to receive(:exist?).with(bar).and_return(true) - - changes = [ - [:file, :modified, foo, 'bar'], - [:file, :removed, foo, 'bar'], - [:file, :added, foo, 'bar'], - [:file, :modified, foo, 'bar'] - ] - allow(config).to receive(:silenced?) { false } - smooshed = subject.smoosh_changes(changes) - expect(smooshed).to eq(modified: ['bar'], added: [], removed: []) - end - it 'ignores deleted temp file' do - bar = instance_double( - Pathname, - to_s: 'bar', - exist?: false) - - foo = instance_double(Pathname, to_s: 'foo', children: []) - allow(foo).to receive(:+).with('bar') { bar } - allow(config).to receive(:exist?).with(bar).and_return(false) - - changes = [ - [:file, :added, foo, 'bar'], - [:file, :modified, foo, 'bar'], - [:file, :removed, foo, 'bar'], - [:file, :modified, foo, 'bar'] - ] - allow(config).to receive(:silenced?) { false } - smooshed = subject.smoosh_changes(changes) - expect(smooshed).to eq(modified: [], added: [], removed: []) - end + allow(dir).to receive(:+).with('foo') { foo } + allow(dir).to receive(:+).with('bar') { bar } + allow(dir).to receive(:+).with('ignored') { ignored } - it 'recognizes double move as modification' do - # e.g. "mv foo x && mv x foo" is like "touch foo" - bar = instance_double( - Pathname, - to_s: 'bar', - exist?: true) - - allow(config).to receive(:exist?).with(bar).and_return(true) - dir = instance_double(Pathname, to_s: 'foo', children: []) - allow(dir).to receive(:+).with('bar') { bar } - - changes = [ - [:file, :removed, dir, 'bar'], - [:file, :added, dir, 'bar'] - ] - allow(config).to receive(:silenced?) { false } - smooshed = subject.smoosh_changes(changes) - expect(smooshed).to eq(modified: ['bar'], added: [], removed: []) - end + allow(config).to receive(:silenced?). + with(Pathname('ignored'), :file) { true } - context 'with cookie' do + allow(config).to receive(:silenced?). + with(Pathname('foo'), :file) { false } - it 'recognizes single moved_to as add' do - foo = instance_double( - Pathname, - to_s: 'foo', - exist?: true) + allow(config).to receive(:silenced?). + with(Pathname('bar'), :file) { false } - dir = instance_double(Pathname, to_s: 'foo', children: []) - allow(dir).to receive(:+).with('foo') { foo } - allow(config).to receive(:exist?).with(foo).and_return(true) - - changes = [[:file, :moved_to, dir, 'foo', cookie: 4321]] - expect(config).to receive(:silenced?). - with(Pathname('foo'), :file) { false } + allow(config).to receive(:exist?).with(foo).and_return(true) + allow(config).to receive(:exist?).with(bar).and_return(true) + allow(config).to receive(:exist?).with(ignored).and_return(true) + end - smooshed = subject.smoosh_changes(changes) - expect(smooshed).to eq(modified: [], added: ['foo'], removed: []) - end + describe 'smoosh_changes' do + subject { described_class.new(config).smoosh_changes(changes) } - it 'recognizes related moved_to as add' do - foo = instance_double( - Pathname, - to_s: 'foo', - exist?: true, - directory?: false) - - bar = instance_double( - Pathname, - to_s: 'bar', - exist?: true, - directory?: false) - - dir = instance_double(Pathname, children: []) - allow(dir).to receive(:+).with('foo') { foo } - allow(dir).to receive(:+).with('bar') { bar } - allow(config).to receive(:exist?).with(foo).and_return(true) - allow(config).to receive(:exist?).with(bar).and_return(true) - - changes = [ - [:file, :moved_from, dir, 'foo', cookie: 4321], - [:file, :moved_to, dir, 'bar', cookie: 4321] + context 'with rename from temp file' do + let(:changes) do + [ + [:file, :modified, dir, 'foo'], + [:file, :removed, dir, 'foo'], + [:file, :added, dir, 'foo'], + [:file, :modified, dir, 'foo'] ] + end + it { is_expected.to eq(modified: ['foo'], added: [], removed: []) } + end - expect(config).to receive(:silenced?). - twice.with(Pathname('foo'), :file) { false } - - expect(config).to receive(:silenced?). - with(Pathname('bar'), :file) { false } + context 'with a deteted temp file' do + before { allow(config).to receive(:exist?).with(foo).and_return(false) } - smooshed = subject.smoosh_changes(changes) - expect(smooshed).to eq(modified: [], added: ['bar'], removed: []) + let(:changes) do + [ + [:file, :added, dir, 'foo'], + [:file, :modified, dir, 'foo'], + [:file, :removed, dir, 'foo'], + [:file, :modified, dir, 'foo'] + ] end + it { is_expected.to eq(modified: [], added: [], removed: []) } + end - # Scenario with workaround for editors using rename() - it 'recognizes related moved_to with ignored moved_from as modify' do - - ignored = instance_double( - Pathname, - to_s: 'ignored', - exist?: true, - directory?: false) - - foo = instance_double( - Pathname, - to_s: 'foo', - exist?: true, - directory?: false) - - dir = instance_double(Pathname, children: []) - allow(dir).to receive(:+).with('foo') { foo } - allow(dir).to receive(:+).with('ignored') { ignored } - - allow(config).to receive(:exist?).with(foo).and_return(true) - allow(config).to receive(:exist?).with(ignored).and_return(true) - - changes = [ - [:file, :moved_from, dir, 'ignored', cookie: 4321], - [:file, :moved_to, dir, 'foo', cookie: 4321] + # e.g. "mv foo x && mv x foo" is like "touch foo" + context 'when double move' do + let(:changes) do + [ + [:file, :removed, dir, 'foo'], + [:file, :added, dir, 'foo'] ] + end + it { is_expected.to eq(modified: ['foo'], added: [], removed: []) } + end - expect(config).to receive(:silenced?). - with(Pathname('ignored'), :file) { true } + context 'with cookie' do + context 'when single moved' do + let(:changes) { [[:file, :moved_to, dir, 'foo', cookie: 4321]] } + it { is_expected.to eq(modified: [], added: ['foo'], removed: []) } + end - expect(config).to receive(:silenced?). - with(Pathname('foo'), :file) { false } + context 'when related moved_to' do + let(:changes) do + [ + [:file, :moved_from, dir, 'foo', cookie: 4321], + [:file, :moved_to, dir, 'bar', cookie: 4321] + ] + end + it { is_expected.to eq(modified: [], added: ['bar'], removed: []) } + end - smooshed = subject.smoosh_changes(changes) - expect(smooshed).to eq(modified: ['foo'], added: [], removed: []) + # Scenario with workaround for editors using rename() + context 'when related moved_to with ignored moved_from' do + let(:changes) do + [ + [:file, :moved_from, dir, 'ignored', cookie: 4321], + [:file, :moved_to, dir, 'foo', cookie: 4321] + ] + end + it { is_expected.to eq(modified: ['foo'], added: [], removed: []) } end end context 'with no cookie' do context 'with ignored file' do - let(:dir) { instance_double(Pathname, children: []) } - let(:ignored) { instance_double(Pathname, to_s: 'foo', exist?: true) } - - before do - expect(config).to receive(:silenced?). - with(Pathname('ignored'), :file) { true } - - allow(dir).to receive(:+).with('ignored') { ignored } - end - - it 'recognizes properly ignores files' do - changes = [[:file, :modified, dir, 'ignored']] - smooshed = subject.smoosh_changes(changes) - expect(smooshed).to eq(modified: [], added: [], removed: []) - end + let(:changes) { [[:file, :modified, dir, 'ignored']] } + it { is_expected.to eq(modified: [], added: [], removed: []) } end end end From d6796047097a877489e2c8313ceea51e86fd8ed4 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Sun, 14 Jun 2015 22:58:10 +0200 Subject: [PATCH 41/47] let acceptance specs bypass Listen API --- spec/support/acceptance_helper.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spec/support/acceptance_helper.rb b/spec/support/acceptance_helper.rb index 4827def5..8731281b 100644 --- a/spec/support/acceptance_helper.rb +++ b/spec/support/acceptance_helper.rb @@ -194,7 +194,8 @@ def listen(reset_queue = true) yield # Polling sleep (default: 1s) - adapter = @listener.send(:adapter) + backend = @listener.instance_variable_get(:@backend) + adapter = backend.instance_variable_get(:@adapter) sleep(1.0) if adapter.is_a?(Listen::Adapter::Polling) # Lag should include: From 937c00885cda3f94e1248683f554cb7e57f1ea42 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Tue, 16 Jun 2015 01:07:12 +0200 Subject: [PATCH 42/47] fix typo --- lib/listen.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/listen.rb b/lib/listen.rb index cde7d130..57d4a1ad 100644 --- a/lib/listen.rb +++ b/lib/listen.rb @@ -4,7 +4,7 @@ require 'listen/internals/thread_pool' -# Always set up logging by default first time file is requried +# Always set up logging by default first time file is required # # NOTE: If you need to clear the logger completely, do so *after* # requiring this file. If you need to set a custom logger, From 7bfdcced65b0e834f25f08ace70998a6283f2756 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Fri, 26 Jun 2015 20:44:34 +0200 Subject: [PATCH 43/47] avoid inspecting inotify events when not debugging --- lib/listen/adapter/linux.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/listen/adapter/linux.rb b/lib/listen/adapter/linux.rb index def22629..e98e6d8b 100644 --- a/lib/listen/adapter/linux.rb +++ b/lib/listen/adapter/linux.rb @@ -45,7 +45,7 @@ def _process_event(dir, event) path = Pathname.new(event.watcher.path) + event.name rel_path = path.relative_path_from(dir).to_s - _log :debug, "inotify: #{rel_path} (#{event.flags.inspect})" + _log(:debug) { "inotify: #{rel_path} (#{event.flags.inspect})" } if /1|true/ =~ ENV['LISTEN_GEM_SIMULATE_FSEVENT'] if (event.flags & [:moved_to, :moved_from]) || _dir_event?(event) From d6ba1b249b024502c90dcf85a97c7f8491392a14 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Fri, 26 Jun 2015 20:45:14 +0200 Subject: [PATCH 44/47] give events time to accumulate in Polling --- lib/listen/adapter/polling.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/listen/adapter/polling.rb b/lib/listen/adapter/polling.rb index ae353aa6..d1d57be6 100644 --- a/lib/listen/adapter/polling.rb +++ b/lib/listen/adapter/polling.rb @@ -8,7 +8,7 @@ module Adapter class Polling < Base OS_REGEXP = // # match every OS - DEFAULTS = { latency: 1.0, wait_for_delay: 0.01 } + DEFAULTS = { latency: 1.0, wait_for_delay: 0.05 } private From 9c573034257db7844e41c7620581acb8044c7be9 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Fri, 26 Jun 2015 20:59:33 +0200 Subject: [PATCH 45/47] bump MRI to 2.2.2 on Travis --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index 089fe540..e313c42d 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ language: ruby bundler_args: --without development rvm: - - 2.2.1 + - 2.2.2 - jruby - rbx-2 matrix: From bf98fc805425bb8044c7ddb2a7d9dc8424712486 Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Fri, 26 Jun 2015 21:00:55 +0200 Subject: [PATCH 46/47] bump RSpec version in Gemfile --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 8c7eb79d..e73d8c46 100644 --- a/Gemfile +++ b/Gemfile @@ -20,7 +20,7 @@ end group :test do gem 'rake' - gem 'rspec', '~> 3.2' + gem 'rspec', '~> 3.3' gem 'rspec-retry' gem 'coveralls' end From 0696aa047f40e929a0e16673764a9b05432144ef Mon Sep 17 00:00:00 2001 From: Cezary Baginski Date: Fri, 26 Jun 2015 21:05:50 +0200 Subject: [PATCH 47/47] remove rspec-retry due to RSpec monkeypatching --- Gemfile | 1 - spec/spec_helper.rb | 2 -- 2 files changed, 3 deletions(-) diff --git a/Gemfile b/Gemfile index e73d8c46..9ab3c001 100644 --- a/Gemfile +++ b/Gemfile @@ -21,7 +21,6 @@ end group :test do gem 'rake' gem 'rspec', '~> 3.3' - gem 'rspec-retry' gem 'coveralls' end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 0a848af6..2d2a8fc4 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -40,10 +40,8 @@ def fake_path(str, options = {}) end end -require 'rspec/retry' RSpec.configure do |config| config.include SpecHelpers - config.default_retry_count = ci? ? 5 : 1 end Thread.abort_on_exception = true