diff --git a/Gemfile b/Gemfile index 665a112..ac1f8a2 100644 --- a/Gemfile +++ b/Gemfile @@ -12,7 +12,9 @@ end gem "hanami-utils", github: "hanami/utils", branch: "main" gem "hanami-cli", github: "hanami/cli", branch: "main" -gem "hanami", github: "hanami/hanami", branch: "main" +# Targets the in-place code reloading branch, which this gem depends on for +# `Hanami::Slice#reload!`. Restore to `branch: "main"` once that has landed. +gem "hanami", github: "hanami/hanami", branch: "internal-code-reloading" gem "hanami-devtools", github: "hanami/devtools", branch: "main" diff --git a/hanami-reloader.gemspec b/hanami-reloader.gemspec index a42d878..a6647e7 100644 --- a/hanami-reloader.gemspec +++ b/hanami-reloader.gemspec @@ -30,9 +30,8 @@ Gem::Specification.new do |spec| spec.required_ruby_version = ">= 3.3" - spec.add_runtime_dependency "guard", "~> 2.19" - spec.add_runtime_dependency "guard-puma", "~> 0.8" spec.add_runtime_dependency "hanami-cli", "~> 3.0.0" + spec.add_runtime_dependency "rack", ">= 3.0" spec.add_runtime_dependency "zeitwerk", "~> 2.6" end diff --git a/lib/hanami/reloader/commands.rb b/lib/hanami/reloader/commands.rb index 8f7ff45..af61b51 100644 --- a/lib/hanami/reloader/commands.rb +++ b/lib/hanami/reloader/commands.rb @@ -5,69 +5,45 @@ module Hanami module Reloader module Commands - # Guardfile - module Guardfile - def self.group - "server" - end - - def self.default_path - path("Guardfile") - end - - def self.path(value) - value - end - end - - # Generate hanami-reloader configuration + # Removes configuration left behind by previous versions of hanami-reloader. + # + # Reloading no longer runs through Guard, so the `Guardfile` it used to generate is now + # dead weight. Nothing is generated in its place: the reloader is wired up by the `server` + # command below, with no per-app configuration. + # + # @api private + # @since 2.1.0 class Install < Hanami::CLI::Command # @api private - # @since 2.1.0 - # - # NOTE: Any change to this constant MUST be reflected in the `#generate_configuration` method, - # by copying and pasting this regex. - MATCHER = %r{^(app|config|lib|slices)([\\/][^\\/]+)*\.(rb|erb|haml|slim)$}i + # @since 3.1.0 + GUARDFILE = "Guardfile" - desc "Generate configuration for code reloading" + desc "Remove obsolete code reloading configuration" def initialize(fs: Dry::Files.new, **args) super end def call(*, **) - generate_configuration(Guardfile.default_path) - end - - private + return unless fs.exist?(GUARDFILE) + return unless fs.read(GUARDFILE).include?("guard \"puma\"") - def generate_configuration(path) - fs.write path, <<~CODE - # frozen_string_literal: true - - group :#{Guardfile.group} do - guard "puma", port: ENV.fetch("#{Hanami::Port::ENV_VAR}", #{Hanami::Port::DEFAULT}), environment: ENV.fetch("HANAMI_ENV", "development") do - # Edit the following regular expression for your needs. - # See: https://hanakai.org/learn/hanami/app/code-reloading/ - watch(%r{^(app|config|lib|slices)([\\/][^\\/]+)*.(rb|erb|haml|slim)$}i) - end - end - CODE + fs.delete(GUARDFILE) + out.puts "Removed #{GUARDFILE} (code reloading no longer uses Guard)" end end - # Override `hanami server` command + # Override `hanami server` to reload the app in place instead of restarting it. + # + # The app is built from `config.ru` here rather than by the Rack server, so that it can be + # wrapped in {Middleware} before being served. That keeps the reloader outside the app's own + # middleware stack, which a reload replaces, and means an app needs no `config.ru` changes to + # get reloading. + # + # @since 2.0.0 + # @api private class Server < Hanami::CLI::Commands::App::Server - # @since 2.0.0 - # @api private - DEFAULT_GUARD_PUMA_OPTIONS = ["-n", "f", "-i", "-g", Guardfile.group, "-G"].freeze - - # @since 2.0.0 - # @api private - OPTIONS_SEPARATOR = " " - - option :guardfile, type: :string, desc: "Path to Guardfile", default: Guardfile.default_path.to_s - option :code_reloading, type: :boolean, desc: "Code reloading", default: true + option :code_reloading, type: :boolean, desc: "Code reloading", default: true desc "Start Hanami app server" @@ -75,11 +51,25 @@ class Server < Hanami::CLI::Commands::App::Server "--no-code-reloading # Disable code reloading" ] - def call(**args) - code_reloading = args.fetch(:code_reloading) + def call(port: Hanami::Port::DEFAULT, **args) + return super(port: port, **args) unless code_reloading?(**args) + + # Keeps HANAMI_PORT in step with an explicit `--port`, then resolves the port the same + # way the command we're replacing does, so a port set in `.env` is still honoured. + Hanami::Port.call!(port) + + reloading_server.call(**args, port: Hanami::Port[port]) + end + + private + + # @api private + # @since 3.1.0 + def code_reloading?(**args) + return false unless args.fetch(:code_reloading) if ENV["HANAMI_ENV"] == "production" - msg = <<~TEXT + err.puts <<~TEXT WARNING: You are running `hanami server` in the production environment via hanami-reloader. Code reloading is disabled, but `hanami server` and hanami-reloader are intended to be used in @@ -88,29 +78,16 @@ def call(**args) For production, start your web server directly, e.g. `bundle exec puma -C config/puma.rb`. TEXT - err.puts msg - - return super + return false end - if code_reloading - guard_puma_env_vars!(**args) - exec "bundle exec guard #{guard_puma_options(**args)}" - else - super - end + true end - private - - def guard_puma_env_vars!(**args) - Hanami::Port.call!(args.fetch(:port)) - end - - def guard_puma_options(**args) - options = DEFAULT_GUARD_PUMA_OPTIONS.dup - options.push(Guardfile.path(args.fetch(:guardfile))) - options.join(OPTIONS_SEPARATOR) + # @api private + # @since 3.1.0 + def reloading_server + Reloader::Server.new(out: out, err: err) end end end diff --git a/lib/hanami/reloader/file_checker.rb b/lib/hanami/reloader/file_checker.rb new file mode 100644 index 0000000..a1b4c1c --- /dev/null +++ b/lib/hanami/reloader/file_checker.rb @@ -0,0 +1,133 @@ +# frozen_string_literal: true + +module Hanami + module Reloader + # Detects changes to an app's source files by comparing their modification times. + # + # Nothing runs in the background: files are stat'd when {#updated?} is called, which the + # reloader does once per request. This needs no native extensions, behaves the same on every + # platform, and means a reload only ever happens between requests. Only the directories Hanami + # loads code from are walked, so the cost is proportional to the app rather than to the project + # (`node_modules/` and friends are never visited). + # + # @api private + # @since 3.1.0 + class FileChecker + # Directories whose contents {Hanami::Slice#reload!} is able to pick up. + # + # @api private + # @since 3.1.0 + WATCHED_DIRS = %w[app config lib slices].freeze + + # @api private + # @since 3.1.0 + WATCHED_EXTENSIONS = %w[rb erb haml slim].freeze + + # Files that a reload cannot apply, because they are loaded once before the app exists. + # + # @api private + # @since 3.1.0 + RESTART_REQUIRED_PATHS = [ + File.join("config", "app.rb"), + "Gemfile", + "Gemfile.lock" + ].freeze + + # @api private + # @since 3.1.0 + attr_reader :root + + # @api private + # @since 3.1.0 + def initialize(root:) + @root = Pathname(root) + @glob = File.join("**", "*.{#{WATCHED_EXTENSIONS.join(",")}}") + @signature = reloadable_signature + @restart_mtimes = restart_required_mtimes + end + + # Returns true if any reloadable file has changed since the last {#commit!}. + # + # This deliberately does not record what it saw. Until {#commit!} is called the change is + # still considered outstanding, so a reload that raises will be attempted again on the next + # check rather than being swallowed. + # + # @return [Boolean] + # + # @api private + # @since 3.1.0 + def updated? + reloadable_signature != @signature + end + + # Accepts the current state of the files as the new baseline. + # + # @api private + # @since 3.1.0 + def commit! + @signature = reloadable_signature + self + end + + # Returns the paths of any changed files that a reload cannot apply. + # + # Unlike {#updated?} this records what it saw, so each change is reported once. + # + # @return [Array] paths relative to the app root, empty if nothing changed + # + # @api private + # @since 3.1.0 + def restart_required + current = restart_required_mtimes + + changed = current.reject { |path, mtime| @restart_mtimes[path] == mtime }.keys + @restart_mtimes = current + + changed + end + + private + + def reloadable_signature + paths = WATCHED_DIRS.flat_map { |dir| Dir.glob(root.join(dir, @glob)) } + + # `config/app.rb` sits inside a watched directory but cannot be applied by a reload, so it + # is excluded here and reported by {#restart_required?} instead. Otherwise every edit to it + # would both warn and trigger a reload that changes nothing. + signature(paths - restart_required_paths) + end + + def restart_required_paths + @restart_required_paths ||= RESTART_REQUIRED_PATHS.map { |path| root.join(path).to_s } + end + + # Keyed by the relative path so a change can be reported by name, and tracked individually so + # that touching one file does not mask a change to another. + def restart_required_mtimes + RESTART_REQUIRED_PATHS.to_h do |path| + [path, File.mtime(root.join(path)).to_f] + rescue Errno::ENOENT + [path, nil] + end + end + + # A file count alongside the newest mtime. Between them these catch the three things that + # matter: a file changing (mtime moves), one being added, and one being deleted (count + # moves). Comparing counts avoids having to keep a hash of every path. + def signature(paths) + count = 0 + latest = 0.0 + + paths.each do |path| + mtime = File.mtime(path).to_f + count += 1 + latest = mtime if mtime > latest + rescue Errno::ENOENT + # Deleted between the glob and the stat; the next check will see a stable state. + end + + [count, latest] + end + end + end +end diff --git a/lib/hanami/reloader/middleware.rb b/lib/hanami/reloader/middleware.rb new file mode 100644 index 0000000..f5f8dd9 --- /dev/null +++ b/lib/hanami/reloader/middleware.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +module Hanami + module Reloader + # Rack middleware that reloads the app in place when its source files change. + # + # This sits *outside* the Hanami app rather than in the app's own middleware stack, because a + # reload replaces everything inside that stack. Wrapping from the outside means the request is + # dispatched into freshly loaded code, instead of into the code that was live when the request + # arrived. + # + # Files are checked once per request rather than watched in the background, so a reload only + # happens when there is something to serve, and never lands halfway through an edit. + # + # @api private + # @since 3.1.0 + class Middleware + # @api private + # @since 3.1.0 + def initialize(app, file_checker:, slice: nil, out: $stdout) + @app = app + @file_checker = file_checker + @slice = slice + @out = out + @mutex = Mutex.new + end + + # @api private + # @since 3.1.0 + def call(env) + @mutex.synchronize { check_for_changes } + + @app.call(env) + end + + private + + # @api private + # @since 3.1.0 + def check_for_changes + restart_required = @file_checker.restart_required + warn_restart_required(restart_required) if restart_required.any? + + return unless @file_checker.updated? + + reload! + + # Only once the reload has succeeded, so a file that raises is retried on the next request + # instead of being silently skipped. + @file_checker.commit! + end + + # @api private + # @since 3.1.0 + def reload! + started = Process.clock_gettime(Process::CLOCK_MONOTONIC) + + slice.reload! + + elapsed = Process.clock_gettime(Process::CLOCK_MONOTONIC) - started + @out.puts("[hanami] Reloaded in #{(elapsed * 1000).round}ms") + end + + # @api private + # @since 3.1.0 + def warn_restart_required(paths) + @out.puts( + "[hanami] #{paths.join(', ')} cannot be reloaded. " \ + "Restart the server to apply your changes." + ) + end + + # Resolved lazily: the middleware is built while the app is being loaded. + # + # @api private + # @since 3.1.0 + def slice + @slice || Hanami.app + end + end + end +end diff --git a/lib/hanami/reloader/server.rb b/lib/hanami/reloader/server.rb new file mode 100644 index 0000000..cbe8bf6 --- /dev/null +++ b/lib/hanami/reloader/server.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +require "hanami/cli/server" +require "rack/builder" + +module Hanami + module Reloader + # Runs the rack server against an app wrapped in {Middleware}. + # + # Hanami's own middleware stack lives inside `Hanami.app`, which a reload replaces wholesale. + # The reloader therefore has to sit outside the app, and the only place outside it - short of + # every app editing its `config.ru` - is between the rack config file being read and the server + # being started. + # + # `Hanami::CLI::Server` hands the rack server a path to `config.ru` and lets it load the file, + # leaving no seam there. Subclassing it opens one while still inheriting its option mapping and + # its choice of rack server, so nothing about how a Hanami app is served is duplicated here. + # + # @api private + # @since 3.1.0 + class Server < Hanami::CLI::Server + # @api private + # @since 3.1.0 + def initialize(out: $stdout, err: $stderr, **opts) + super(**opts) + @out = out + @err = err + end + + # @api private + # @since 3.1.0 + def call(**options) + rack_options = Hash[ + extract_rack_fallback_options(options) + extract_overriding_options(options) + ] + + # Read here rather than by the caller, so a `--config` option is honoured. + rack_options[:app] = wrap(Rack::Builder.parse_file(rack_options.fetch(:config))) + + rack_server.start(rack_options) + end + + private + + # The app is only available once the rack config file has been read, so this is the first + # point at which its config can be consulted. + # + # @api private + # @since 3.1.0 + def wrap(app) + unless Hanami.app.config.code_reloading + @err.puts( + "WARNING: `config.code_reloading` is false, so the app cannot be reloaded. " \ + "Starting without code reloading." + ) + return app + end + + Middleware.new(app, file_checker: FileChecker.new(root: Hanami.app.root), out: @out) + end + end + end +end diff --git a/spec/unit/hanami/reloader/commands/install_spec.rb b/spec/unit/hanami/reloader/commands/install_spec.rb index 5b933d9..b86aade 100644 --- a/spec/unit/hanami/reloader/commands/install_spec.rb +++ b/spec/unit/hanami/reloader/commands/install_spec.rb @@ -4,14 +4,11 @@ RSpec.describe Hanami::Reloader::Commands::Install do describe "#call" do - subject { described_class.new(fs: fs) } + subject { described_class.new(fs: fs, out: out) } let(:fs) { Dry::Files.new } + let(:out) { StringIO.new } let(:dir) { Dir.mktmpdir } - let(:app) { "synth" } - let(:app_name) { "Synth" } - - let(:arbitrary_argument) { {} } around do |example| fs.chdir(dir) { example.run } @@ -19,24 +16,41 @@ fs.delete_directory(dir) end - it "generates configurations" do - subject.call(arbitrary_argument) + context "when a Guard-based Guardfile is present" do + before do + fs.write("Guardfile", <<~RUBY) + group :server do + guard "puma", port: 2300 do + watch(%r{^app/.*\\.rb$}) + end + end + RUBY + end - # Guardfile - matcher = Hanami::Reloader::Commands::Install::MATCHER.inspect.gsub("/^", "^").gsub("$/i", "$}i").gsub('*\\.', "*.").gsub("\\\\\\", %(\\)) - matcher = %(%r{#{matcher}) - guardfile = <<~EOF - # frozen_string_literal: true + it "removes it, since reloading no longer runs through Guard" do + subject.call({}) - group :server do - guard "puma", port: ENV.fetch("HANAMI_PORT", 2300), environment: ENV.fetch("HANAMI_ENV", "development") do - # Edit the following regular expression for your needs. - # See: https://hanakai.org/learn/hanami/app/code-reloading/ - watch(#{matcher}) - end - end - EOF - expect(fs.read("Guardfile")).to eq(guardfile) + expect(fs.exist?("Guardfile")).to be(false) + expect(out.string).to include("Removed Guardfile") + end + end + + context "when a Guardfile is present but not ours" do + before { fs.write("Guardfile", "guard \"rspec\" do\nend\n") } + + it "leaves it alone" do + subject.call({}) + + expect(fs.exist?("Guardfile")).to be(true) + end + end + + context "when no Guardfile is present" do + it "does nothing" do + expect { subject.call({}) }.not_to raise_error + + expect(fs.exist?("Guardfile")).to be(false) + end end end end diff --git a/spec/unit/hanami/reloader/commands/server_spec.rb b/spec/unit/hanami/reloader/commands/server_spec.rb index 1112df8..261a562 100644 --- a/spec/unit/hanami/reloader/commands/server_spec.rb +++ b/spec/unit/hanami/reloader/commands/server_spec.rb @@ -1,155 +1,118 @@ # frozen_string_literal: true -require "pathname" +require "tmpdir" +require "fileutils" RSpec.describe Hanami::Reloader::Commands::Server do - describe "#call" do - before { ENV.delete("HANAMI_PORT") } - - let(:args) { {code_reloading: code_reloading, guardfile: guardfile, port: port} } - let(:code_reloading) { true } - let(:guardfile) { Hanami::Reloader::Commands::Guardfile.default_path } - let(:port) { 2300 } - let(:err) { StringIO.new } - - context "when in production env" do - before { ENV["HANAMI_ENV"] = "production" } - after { ENV.delete("HANAMI_ENV") } + subject(:command) { described_class.new(server: server, out: out, err: err) } - it "prints a warning when running hanami server in production" do - allow_any_instance_of(described_class).to receive(:err).and_return(err) - server = described_class.new(server: proc { |*| }) - - expect(err).to receive(:puts).with(a_string_including("WARNING: You are running `hanami server` in the production environment via hanami-reloader.")) - - server.call(**args) - end + let(:captured) { {} } + let(:server) do + spy("server").tap { |s| allow(s).to receive(:call) { |**kwargs| captured.replace(kwargs) } } + end + let(:out) { StringIO.new } + let(:err) { StringIO.new } + + let(:args) { {code_reloading: code_reloading, port: port} } + let(:code_reloading) { true } + let(:port) { Hanami::Port::DEFAULT } + + before { ENV.delete("HANAMI_PORT") } + after { ENV.delete("HANAMI_PORT") } + + # The reloading runner is covered by its own spec; here we only care that the command reaches + # for it, and with which options. + let(:reloading_server) { spy("reloading server") } + before do + allow(command).to receive(:reloading_server).and_return(reloading_server) + allow(reloading_server).to receive(:call) { |**kwargs| captured.replace(kwargs) } + end - it "does not start guard despite code_reloading being enabled" do - server = described_class.new(server: proc { |*| }) - expect(server).to_not receive(:exec).with("bundle exec guard -n f -i -g server -G Guardfile") + describe "#call" do + context "with code reloading enabled" do + it "serves through the reloading runner rather than the plain one" do + command.call(**args) - server.call(**args) + expect(reloading_server).to have_received(:call) + expect(server).not_to have_received(:call) end - end - context "with code reloading enabled" do - context "with default arguments" do - it "starts server" do - allow(subject).to receive(:exec).with("bundle exec guard -n f -i -g server -G Guardfile") + it "does not shell out to Guard" do + expect(command).not_to receive(:exec) - subject.call(**args) - end + command.call(**args) end - context "without .env port" do - it "doesn't set HANAMI_PORT" do - allow(subject).to receive(:exec) - subject.call(**args) + context "without a port in the environment" do + it "does not set HANAMI_PORT" do + command.call(**args) expect(ENV.fetch("HANAMI_PORT", nil)).to be(nil) end - context "with custom port CLI option" do + context "with a custom port CLI option" do let(:port) { 9000 } - it "sets HANAMI_PORT value" do - allow(subject).to receive(:exec) - subject.call(**args) + it "sets HANAMI_PORT and serves on that port" do + command.call(**args) - expect(ENV.fetch("HANAMI_PORT", nil)).to eq(port.to_s) + expect(ENV.fetch("HANAMI_PORT", nil)).to eq("9000") + expect(captured[:port]).to eq(9000) end end end - context "with .env port" do - before { ENV["HANAMI_PORT"] = dotenv_port.to_s } - let(:dotenv_port) { 9000 } + context "with a port in the environment" do + before { ENV["HANAMI_PORT"] = "9000" } - it "respects HANAMI_PORT value" do - allow(subject).to receive(:exec) - subject.call(**args) + it "serves on the environment's port" do + command.call(**args) - expect(ENV.fetch("HANAMI_PORT", nil)).to eq(dotenv_port.to_s) + expect(ENV.fetch("HANAMI_PORT", nil)).to eq("9000") + expect(captured[:port]).to eq(9000) end - context "with custom port CLI option" do + context "with a custom port CLI option" do let(:port) { 18_000 } - let(:cli_arg_port) { port } - it "overrides HANAMI_PORT value" do - allow(subject).to receive(:exec) - subject.call(**args) + it "lets the CLI option win" do + command.call(**args) - expect(ENV.fetch("HANAMI_PORT", nil)).to eq(cli_arg_port.to_s) + expect(ENV.fetch("HANAMI_PORT", nil)).to eq("18000") + expect(captured[:port]).to eq(18_000) end end end end context "with code reloading disabled" do - subject { described_class.new(server: server) } let(:code_reloading) { false } - let(:server) { proc { |*| } } - - it "starts original hanami-cli server" do - allow(server).to receive(:call) - - subject.call(**args) - end - - context "without .env port" do - it "doesn't set HANAMI_PORT" do - allow(server).to receive(:call) - subject.call(**args) - expect(ENV.fetch("HANAMI_PORT", nil)).to be(nil) - end - - context "with custom port CLI option" do - let(:port) { 9000 } - - it "doesn't set HANAMI_PORT" do - allow(server).to receive(:call) - subject.call(**args) + it "serves through the plain runner" do + command.call(**args) - expect(ENV.fetch("HANAMI_PORT", nil)).to be(nil) - end - end + expect(server).to have_received(:call) + expect(reloading_server).not_to have_received(:call) end + end - context "with .env port" do - before { ENV["HANAMI_PORT"] = dotenv_port.to_s } - let(:dotenv_port) { 9000 } - - it "respects HANAMI_PORT value" do - allow(server).to receive(:call) - subject.call(**args) - - expect(ENV.fetch("HANAMI_PORT", nil)).to eq(dotenv_port.to_s) - end - - context "with custom port CLI option" do - let(:port) { 18_000 } - let(:cli_arg_port) { port } + context "in the production environment" do + before { ENV["HANAMI_ENV"] = "production" } + after { ENV.delete("HANAMI_ENV") } - it "respects HANAMI_PORT value" do - allow(server).to receive(:call) - subject.call(**args) + it "warns" do + command.call(**args) - expect(ENV.fetch("HANAMI_PORT", nil)).to eq(dotenv_port.to_s) - end - end + expect(err.string).to include( + "WARNING: You are running `hanami server` in the production environment via hanami-reloader." + ) end - end - - context "with custom Guardfile path" do - let(:guardfile) { Pathname.new(Dir.pwd).join("Guardfile") } - it "uses given value" do - allow(subject).to receive(:exec).with("bundle exec guard -n f -i -g server -G #{guardfile}") + it "serves through the plain runner, despite code reloading being enabled" do + command.call(**args) - subject.call(**args) + expect(server).to have_received(:call) + expect(reloading_server).not_to have_received(:call) end end end diff --git a/spec/unit/hanami/reloader/file_checker_spec.rb b/spec/unit/hanami/reloader/file_checker_spec.rb new file mode 100644 index 0000000..bf2c03a --- /dev/null +++ b/spec/unit/hanami/reloader/file_checker_spec.rb @@ -0,0 +1,141 @@ +# frozen_string_literal: true + +require "tmpdir" +require "fileutils" + +RSpec.describe Hanami::Reloader::FileChecker do + subject(:file_checker) { described_class.new(root: dir) } + + let(:dir) { Pathname(Dir.mktmpdir) } + + after { FileUtils.remove_entry(dir) } + + def write(path, content = "# frozen_string_literal: true\n") + full = dir.join(path) + full.dirname.mkpath + full.write(content) + full + end + + # mtime has one-second granularity on some filesystems, so move times explicitly rather than + # relying on the clock advancing between writes. + def touch(path, offset: 10) + full = dir.join(path) + time = Time.now + offset + File.utime(time, time, full) + end + + before { write("app/greeter.rb") } + + describe "#updated?" do + it "is false when nothing has changed" do + expect(file_checker.updated?).to be(false) + end + + it "is true when a watched file is modified" do + file_checker + touch("app/greeter.rb") + + expect(file_checker.updated?).to be(true) + end + + it "is true when a watched file is added" do + file_checker + write("app/farewell.rb") + touch("app/farewell.rb") + + expect(file_checker.updated?).to be(true) + end + + it "is true when a watched file is deleted" do + file_checker + FileUtils.rm(dir.join("app/greeter.rb")) + + expect(file_checker.updated?).to be(true) + end + + it "watches config, lib and slices as well as app" do + %w[config/routes.rb lib/thing.rb slices/main/action.rb].each do |path| + w = described_class.new(root: dir) + write(path) + touch(path) + + expect(w.updated?).to be(true), "expected a change in #{path} to be seen" + end + end + + it "sees template files, not just Ruby" do + file_checker + write("app/templates/home.html.erb", "

hi

") + touch("app/templates/home.html.erb") + + expect(file_checker.updated?).to be(true) + end + + it "ignores files outside the watched directories" do + file_checker + write("node_modules/pkg/index.rb") + write("public/assets/app.rb") + touch("node_modules/pkg/index.rb") + touch("public/assets/app.rb") + + expect(file_checker.updated?).to be(false) + end + + it "does not treat config/app.rb as reloadable, since a reload cannot apply it" do + write("config/app.rb") + w = described_class.new(root: dir) + touch("config/app.rb") + + expect(w.updated?).to be(false) + expect(w.restart_required).to eq(["config/app.rb"]) + end + + it "keeps reporting a change until it is committed" do + file_checker + touch("app/greeter.rb") + + expect(file_checker.updated?).to be(true) + expect(file_checker.updated?).to be(true) + + file_checker.commit! + + expect(file_checker.updated?).to be(false) + end + end + + describe "#restart_required" do + before { write("config/app.rb") } + + it "is empty when nothing has changed" do + expect(file_checker.restart_required).to be_empty + end + + it "names the changed file, and reports it once" do + file_checker + touch("config/app.rb") + + expect(file_checker.restart_required).to eq(["config/app.rb"]) + expect(file_checker.restart_required).to be_empty + end + + it "reports a change to the Gemfile" do + write("Gemfile") + checker = described_class.new(root: dir) + touch("Gemfile") + + expect(checker.restart_required).to eq(["Gemfile"]) + end + + it "tracks each file separately, so one change does not mask another" do + write("Gemfile") + checker = described_class.new(root: dir) + + touch("config/app.rb", offset: 10) + expect(checker.restart_required).to eq(["config/app.rb"]) + + touch("Gemfile", offset: 20) + expect(checker.restart_required).to eq(["Gemfile"]) + end + end +end diff --git a/spec/unit/hanami/reloader/matcher_spec.rb b/spec/unit/hanami/reloader/matcher_spec.rb deleted file mode 100644 index 7fe80ef..0000000 --- a/spec/unit/hanami/reloader/matcher_spec.rb +++ /dev/null @@ -1,105 +0,0 @@ -# frozen_string_literal: true - -require "hanami/reloader/commands" - -RSpec.describe "Hanami::Reloader::Commands::Install::MATCHER" do - subject { Hanami::Reloader::Commands::Install::MATCHER } - - let(:matching_paths) do - [ - "app/action.rb", - "app/view.rb", - "app/actions/books/index.rb", - "app/actions/books/discounted/index.rb", - "app/views/helpers.rb", - "app/views/books/index.rb", - "app/views/books/discounted/index.rb", - "app/templates/books/index.html.erb", - "app/templates/books/discounted/index.html.erb", - "app/templates/layouts/app.html.erb", - "config/app.rb", - "config/puma.rb", - "config/routes.rb", - "config/settings.rb", - "lib/bookshelf/types.rb", - "slices/admin/action.rb", - "slices/admin/view.rb", - "slices/admin/actions/users/index.rb", - "slices/admin/actions/users/deactivated/index.rb", - "slices/admin/views/helpers.rb", - "slices/admin/views/users/index.rb", - "slices/admin/views/users/deactivated/index.rb", - "slices/admin/templates/users/index.html.slim", - "slices/admin/templates/users/deactivated/index.html.slim", - "slices/api/templates/authors/index.json.haml", - "slices/api/templates/authors/uprising/index.json.haml" - ] - end - - let(:non_matching_paths) do - [ - "Gemfile", - "Gemfile.lock", - "Guardfile", - "Procfile.dev", - "README.md", - "Rakefile", - "config.ru", - "app/assets/css/app.css", - "app/assets/js/app.js", - "app/assets/images/favicon.ico", - "slices/admin/assets/css/app.css", - "slices/admin/assets/js/app.js", - "slices/admin/assets/images/favicon.ico", - "log/test.log", - "node_modules/hanami-assets/dist/hanami-assets.js", - "package.json", - "package-lock.json", - "package.json", - "public/assets.json", - "public/assets/favicon.ico", - "spec/spec_helper.rb", - "spec/requests/root_spec.rb", - "spec/support/requests.rb", - "spec/support/rspec.rb", - "spec/actions/books/index_spec.rb", - "spec/actions/books/discounted/index_spec.rb", - "spec/views/books/index_spec.rb", - "spec/views/books/discounted/index_spec.rb" - ] - end - - context "UNIX" do - it "matches the given patterns" do - matching_paths.each do |path| - expect(path).to match(subject) - end - end - - it "does not matches the given patterns" do - non_matching_paths.each do |path| - expect(path).to_not match(subject) - end - end - end - - context "Windows" do - it "matches the given patterns" do - matching_paths.each do |path| - expect(windows_path(path)).to match(subject) - end - end - - it "does not matches the given patterns" do - non_matching_paths.each do |path| - expect(windows_path(path)).to_not match(subject) - end - end - - private - - def windows_path(path) - path.gsub("/", "\\") - end - end -end diff --git a/spec/unit/hanami/reloader/middleware_spec.rb b/spec/unit/hanami/reloader/middleware_spec.rb new file mode 100644 index 0000000..1c94232 --- /dev/null +++ b/spec/unit/hanami/reloader/middleware_spec.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true + +RSpec.describe Hanami::Reloader::Middleware do + subject(:middleware) do + described_class.new(inner, file_checker: file_checker, slice: slice, out: out) + end + + let(:inner) { ->(_env) { [200, {}, ["ok"]] } } + let(:out) { StringIO.new } + let(:env) { {"PATH_INFO" => "/"} } + + # Stands in for a slice class (e.g. `Hanami.app`), which responds to `reload!`. + let(:slice) { double("slice", reload!: true) } + + let(:file_checker) do + instance_double( + Hanami::Reloader::FileChecker, + updated?: updated, restart_required: restart_required, commit!: true + ) + end + let(:updated) { false } + let(:restart_required) { [] } + + it "passes the request through" do + expect(middleware.call(env)).to eq([200, {}, ["ok"]]) + end + + context "when nothing has changed" do + it "does not reload" do + expect(slice).not_to receive(:reload!) + + middleware.call(env) + end + end + + context "when a watched file has changed" do + let(:updated) { true } + + it "reloads before dispatching" do + order = [] + allow(slice).to receive(:reload!) { order << :reload } + app = described_class.new( + ->(_env) { order << :dispatch; [200, {}, ["ok"]] }, + file_checker: file_checker, slice: slice, out: out + ) + + app.call(env) + + expect(order).to eq([:reload, :dispatch]) + end + + it "commits the file_checker so the change is not reloaded twice" do + expect(file_checker).to receive(:commit!) + + middleware.call(env) + end + + it "reports how long the reload took" do + middleware.call(env) + + expect(out.string).to match(/\[hanami\] Reloaded in \d+ms/) + end + + context "and the reload raises" do + before { allow(slice).to receive(:reload!).and_raise(SyntaxError, "unexpected end") } + + it "lets the error surface" do + expect { middleware.call(env) }.to raise_error(SyntaxError) + end + + it "does not commit, so the next request retries the reload" do + expect(file_checker).not_to receive(:commit!) + + expect { middleware.call(env) }.to raise_error(SyntaxError) + end + end + end + + context "when a file requiring a restart has changed" do + let(:restart_required) { ["config/app.rb"] } + + it "warns instead of silently doing nothing" do + middleware.call(env) + + expect(out.string).to include("config/app.rb cannot be reloaded") + expect(out.string).to include("Restart the server") + end + + it "still serves the request" do + expect(middleware.call(env)).to eq([200, {}, ["ok"]]) + end + end + + context "with concurrent requests" do + let(:updated) { true } + + it "reloads only once" do + reloads = 0 + mutex = Mutex.new + allow(slice).to receive(:reload!) { mutex.synchronize { reloads += 1 } } + + # After the first reload commits, the file_checker reports no further change. + allow(file_checker).to receive(:updated?).and_return(true, false, false, false) + + app = described_class.new(inner, file_checker: file_checker, slice: slice, out: out) + 4.times.map { Thread.new { app.call(env) } }.each(&:join) + + expect(reloads).to eq(1) + end + end +end diff --git a/spec/unit/hanami/reloader/server_spec.rb b/spec/unit/hanami/reloader/server_spec.rb new file mode 100644 index 0000000..5c5aa0d --- /dev/null +++ b/spec/unit/hanami/reloader/server_spec.rb @@ -0,0 +1,77 @@ +# frozen_string_literal: true + +require "tmpdir" +require "fileutils" + +RSpec.describe Hanami::Reloader::Server do + subject(:server) { described_class.new(rack_server: rack_server, out: out, err: err) } + + let(:rack_server) do + Class.new do + attr_reader :options + + def start(options) = @options = options + end.new + end + + let(:out) { StringIO.new } + let(:err) { StringIO.new } + let(:dir) { Pathname(Dir.mktmpdir) } + + let(:code_reloading) { true } + + before do + dir.join("config.ru").write(<<~RUBY) + run ->(_env) { [200, {}, ["from config.ru"]] } + RUBY + + allow(Hanami).to receive(:app).and_return( + double("app", root: dir, config: double("config", code_reloading: code_reloading)) + ) + end + + after { FileUtils.remove_entry(dir) } + + def call(**options) + Dir.chdir(dir) { server.call(config: "config.ru", **options) } + end + + it "inherits the option mapping from Hanami::CLI::Server" do + call(host: "127.0.0.3", port: 2000, debug: true, warn: false) + + expect(rack_server.options[:Host]).to eq("127.0.0.3") + expect(rack_server.options[:Port]).to be(2000) + expect(rack_server.options[:debug]).to be(true) + end + + it "serves the app from config.ru, wrapped in the reloader middleware" do + call + + app = rack_server.options[:app] + + expect(app).to be_a(Hanami::Reloader::Middleware) + expect(app.call({})).to eq([200, {}, ["from config.ru"]]) + end + + it "reads the config file given by the config option" do + dir.join("custom.ru").write(<<~RUBY) + run ->(_env) { [200, {}, ["from custom.ru"]] } + RUBY + + Dir.chdir(dir) { server.call(config: "custom.ru") } + + expect(rack_server.options[:app].call({})).to eq([200, {}, ["from custom.ru"]]) + end + + context "when the app has code reloading disabled" do + let(:code_reloading) { false } + + it "warns and serves the app unwrapped" do + call + + expect(err.string).to include("`config.code_reloading` is false") + expect(rack_server.options[:app]).not_to be_a(Hanami::Reloader::Middleware) + expect(rack_server.options[:app].call({})).to eq([200, {}, ["from config.ru"]]) + end + end +end