From 5a356c1c13f79f903328dd4857eda4254ad08f4c Mon Sep 17 00:00:00 2001 From: Matt Brictson Date: Fri, 16 Feb 2024 17:25:10 -0800 Subject: [PATCH 1/4] Impl simple algorithm for finding test file for given path --- .rubocop.yml | 4 ++ lib/mighty_test.rb | 1 + lib/mighty_test/file_system.rb | 11 ++++ .../fixtures/rails_project/app/models/user.rb | 0 .../rails_project/test/models/user_test.rb | 0 test/mighty_test/file_system_test.rb | 50 +++++++++++++++++++ 6 files changed, 66 insertions(+) create mode 100644 lib/mighty_test/file_system.rb create mode 100644 test/fixtures/rails_project/app/models/user.rb create mode 100644 test/fixtures/rails_project/test/models/user_test.rb create mode 100644 test/mighty_test/file_system_test.rb diff --git a/.rubocop.yml b/.rubocop.yml index e09e54a..d5daca2 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -48,6 +48,10 @@ Layout/MultilineMethodCallIndentation: Layout/SpaceAroundEqualsInParameterDefault: EnforcedStyle: no_space +Lint/EmptyFile: + Exclude: + - "test/fixtures/**/*" + Metrics/AbcSize: Max: 20 Exclude: diff --git a/lib/mighty_test.rb b/lib/mighty_test.rb index d20b6fe..a891eb1 100644 --- a/lib/mighty_test.rb +++ b/lib/mighty_test.rb @@ -1,6 +1,7 @@ module MightyTest autoload :VERSION, "mighty_test/version" autoload :CLI, "mighty_test/cli" + autoload :FileSystem, "mighty_test/file_system" autoload :MinitestRunner, "mighty_test/minitest_runner" autoload :OptionParser, "mighty_test/option_parser" autoload :TestParser, "mighty_test/test_parser" diff --git a/lib/mighty_test/file_system.rb b/lib/mighty_test/file_system.rb new file mode 100644 index 0000000..437816e --- /dev/null +++ b/lib/mighty_test/file_system.rb @@ -0,0 +1,11 @@ +module MightyTest + class FileSystem + def find_matching_test_file(path) + return nil unless path && File.exist?(path) && !Dir.exist?(path) + return path if path.match?(%r{^test/.*_test.rb$}) + + test_path = path[%r{^(?:app|lib)/(.+)\.[^\.]+$}, 1].then { "test/#{_1}_test.rb" } + test_path if test_path && File.exist?(test_path) + end + end +end diff --git a/test/fixtures/rails_project/app/models/user.rb b/test/fixtures/rails_project/app/models/user.rb new file mode 100644 index 0000000..e69de29 diff --git a/test/fixtures/rails_project/test/models/user_test.rb b/test/fixtures/rails_project/test/models/user_test.rb new file mode 100644 index 0000000..e69de29 diff --git a/test/mighty_test/file_system_test.rb b/test/mighty_test/file_system_test.rb new file mode 100644 index 0000000..6d7b6da --- /dev/null +++ b/test/mighty_test/file_system_test.rb @@ -0,0 +1,50 @@ +require "test_helper" + +module MightyTest + class FileSystemTest < Minitest::Test + include FixturesPath + + def test_find_matching_test_file_returns_nil_for_nil_path + assert_nil find_matching_test_file(nil) + end + + def test_find_matching_test_file_returns_nil_for_non_existent_path + assert_nil find_matching_test_file("path/to/nowhere.rb") + end + + def test_find_matching_test_file_returns_nil_for_directory_path + assert_nil find_matching_test_file("lib/example", in: fixtures_path.join("example_project")) + end + + def test_find_matching_test_file_returns_nil_for_path_with_no_corresponding_test + assert_nil find_matching_test_file("lib/example/version.rb", in: fixtures_path.join("example_project")) + end + + def test_find_matching_test_file_returns_nil_for_a_test_support_file + assert_nil find_matching_test_file("test/test_helper.rb", in: fixtures_path.join("example_project")) + end + + def test_find_matching_test_file_returns_argument_if_it_is_already_a_test + test_path = find_matching_test_file("test/example_test.rb", in: fixtures_path.join("example_project")) + assert_equal("test/example_test.rb", test_path) + end + + def test_find_matching_test_file_returns_matching_test_given_an_implementation_path_in_a_gem_project + test_path = find_matching_test_file("lib/example.rb", in: fixtures_path.join("example_project")) + assert_equal("test/example_test.rb", test_path) + end + + def test_find_matching_test_file_returns_matching_test_given_a_model_path_in_a_rails_project + test_path = find_matching_test_file("app/models/user.rb", in: fixtures_path.join("rails_project")) + assert_equal("test/models/user_test.rb", test_path) + end + + private + + def find_matching_test_file(path, in: ".") + Dir.chdir(binding.local_variable_get(:in)) do + FileSystem.new.find_matching_test_file(path) + end + end + end +end From 64ebbc1328add22e3def78cb64f003c54e1acbe7 Mon Sep 17 00:00:00 2001 From: Matt Brictson Date: Sun, 18 Feb 2024 13:12:04 -0800 Subject: [PATCH 2/4] WIP --- lib/mighty_test.rb | 1 + lib/mighty_test/cli.rb | 6 ++ lib/mighty_test/file_system.rb | 5 ++ lib/mighty_test/option_parser.rb | 6 +- lib/mighty_test/watcher.rb | 75 +++++++++++++++++ mighty_test.gemspec | 2 + test/integration/mt_test.rb | 22 +++++ test/mighty_test/watcher_test.rb | 140 +++++++++++++++++++++++++++++++ 8 files changed, 255 insertions(+), 2 deletions(-) create mode 100644 lib/mighty_test/watcher.rb create mode 100644 test/mighty_test/watcher_test.rb diff --git a/lib/mighty_test.rb b/lib/mighty_test.rb index a891eb1..e1d62d7 100644 --- a/lib/mighty_test.rb +++ b/lib/mighty_test.rb @@ -5,4 +5,5 @@ module MightyTest autoload :MinitestRunner, "mighty_test/minitest_runner" autoload :OptionParser, "mighty_test/option_parser" autoload :TestParser, "mighty_test/test_parser" + autoload :Watcher, "mighty_test/watcher" end diff --git a/lib/mighty_test/cli.rb b/lib/mighty_test/cli.rb index 551c40a..5f08a32 100644 --- a/lib/mighty_test/cli.rb +++ b/lib/mighty_test/cli.rb @@ -13,6 +13,8 @@ def run(argv: ARGV) print_help elsif options[:version] puts VERSION + elsif options[:watch] + watch elsif path_args.grep(/.:\d+$/).any? run_test_by_line_number else @@ -33,6 +35,10 @@ def print_help runner.print_help_and_exit! end + def watch + Watcher.new(extra_args:).run + end + def run_test_by_line_number path, line = path_args.first.match(/^(.+):(\d+)$/).captures test_name = TestParser.new(path).test_name_at_line(line.to_i) diff --git a/lib/mighty_test/file_system.rb b/lib/mighty_test/file_system.rb index 437816e..222cc9f 100644 --- a/lib/mighty_test/file_system.rb +++ b/lib/mighty_test/file_system.rb @@ -1,5 +1,10 @@ module MightyTest class FileSystem + def listen(&) + require "listen" + Listen.to(*%w[app lib test].select { |p| Dir.exist?(p) }, relative: true, &).tap(&:start) + end + def find_matching_test_file(path) return nil unless path && File.exist?(path) && !Dir.exist?(path) return path if path.match?(%r{^test/.*_test.rb$}) diff --git a/lib/mighty_test/option_parser.rb b/lib/mighty_test/option_parser.rb index f107214..d79b380 100644 --- a/lib/mighty_test/option_parser.rb +++ b/lib/mighty_test/option_parser.rb @@ -1,15 +1,17 @@ +require "optparse" + module MightyTest class OptionParser def initialize - require "optparse" - @parser = ::OptionParser.new do |op| op.require_exact = true op.banner = <<~BANNER Usage: mt ... + mt --watch BANNER + op.on("--watch") { options[:watch] = true } op.on("-h", "--help") { options[:help] = true } op.on("--version") { options[:version] = true } end diff --git a/lib/mighty_test/watcher.rb b/lib/mighty_test/watcher.rb new file mode 100644 index 0000000..2a825b9 --- /dev/null +++ b/lib/mighty_test/watcher.rb @@ -0,0 +1,75 @@ +require "concurrent" + +module MightyTest + class Watcher + WATCHING_FOR_CHANGES = "Watching for changes to source and test files. Press ctrl-c to exit.".freeze + + def initialize(extra_args: [], file_system: FileSystem.new, system_proc: method(:system)) + @dispatcher = Concurrent::MVar.new + @extra_args = extra_args + @file_system = file_system + @system_proc = system_proc + end + + def run + start_listener + puts WATCHING_FOR_CHANGES + process_dispatched_events + ensure + listener&.stop + end + + # Normally `run` proceeds indefinitely until the user interrupts with ctrl-c. + # This is a way to stop it gracefully in unit tests. + def interrupt + dispatch { :stop } + end + + private + + attr_reader :extra_args, :file_system, :listener, :system_proc + + def file_system_changed(modified, added, _removed) + test_paths = [*modified, *added].filter_map do |path| + file_system.find_matching_test_file(path) + end + return if test_paths.empty? + + run_tests(*test_paths.uniq) + end + + def run_tests(*test_paths) + begin + system_proc.call("mt", *extra_args, "--", *test_paths.flatten) + rescue Interrupt + # Pressing ctrl-c kills the fs_event background process, so we have to manually restart it. + restart_listener + end + puts WATCHING_FOR_CHANGES + end + + def start_listener + listener.stop if listener && !listener.stopped? + @listener = file_system.listen do |*args| + listener.pause + dispatch do + file_system_changed(*args) + listener.start if listener.paused? + end + end + end + alias restart_listener start_listener + + def dispatch(&block) + @dispatcher.put(block) + true + end + + def process_dispatched_events + loop do + result = @dispatcher.take.call + break if result == :stop + end + end + end +end diff --git a/mighty_test.gemspec b/mighty_test.gemspec index 210f59c..624ef9a 100644 --- a/mighty_test.gemspec +++ b/mighty_test.gemspec @@ -26,6 +26,8 @@ Gem::Specification.new do |spec| spec.require_paths = ["lib"] # Runtime dependencies + spec.add_dependency "concurrent-ruby", "~> 1.1" + spec.add_dependency "listen", "~> 3.5" spec.add_dependency "minitest", "~> 5.15" spec.add_dependency "minitest-fail-fast", "~> 0.1.0" spec.add_dependency "minitest-focus", "~> 1.4" diff --git a/test/integration/mt_test.rb b/test/integration/mt_test.rb index 370a74e..fa02c2f 100644 --- a/test/integration/mt_test.rb +++ b/test/integration/mt_test.rb @@ -57,6 +57,28 @@ def test_mt_runs_no_tests_if_line_number_doesnt_match refute_match(/FailingTest/, result.stdout) end + def test_mt_runs_watch_mode_that_executes_tests_when_files_change + project_dir = fixtures_path.join("example_project") + stdout, = capture_subprocess_io do + # Start mt --watch in the background + pid = spawn(*%w[bundle exec mt --watch --verbose], chdir: project_dir) + + # mt needs time to launch and start its file system listener + sleep 1 + + # Touch a file and wait for mt --watch to detect the change and run the corresponding test + FileUtils.touch project_dir.join("lib/example.rb") + sleep 1 + + # OK, we're done here. Tell mt --watch to exit. + Process.kill(:INT, pid) + end + + assert_includes(stdout, "Watching for changes to source and test files.") + assert_match(/ExampleTest/, stdout) + assert_match(/\d runs, \d assertions, 0 failures, 0 errors/, stdout) + end + private def bundle_exec_mt(argv:, env: { "CI" => nil }, chdir: nil, raise_on_failure: true) diff --git a/test/mighty_test/watcher_test.rb b/test/mighty_test/watcher_test.rb new file mode 100644 index 0000000..58df3b2 --- /dev/null +++ b/test/mighty_test/watcher_test.rb @@ -0,0 +1,140 @@ +require "test_helper" + +module MightyTest + class WatcherTest < Minitest::Test + include FixturesPath + + def test_watcher_passes_unique_set_of_test_files_to_mt_command_based_on_changes_detected + system_proc { |*args| puts "[SYSTEM] #{args.join(' ')}" } + listen_thread do |callback| + callback.call(["lib/example.rb", "test/focused_test.rb"], ["test/focused_test.rb"], []) + @watcher.interrupt + end + + stdout, = run_watcher(in: fixtures_path.join("example_project")) + + assert_includes(stdout, "[SYSTEM] mt -- test/example_test.rb test/focused_test.rb\n") + end + + def test_watcher_passes_extra_args_through_to_mt_command + system_proc { |*args| puts "[SYSTEM] #{args.join(' ')}" } + listen_thread do |callback| + callback.call(["test/example_test.rb"], [], []) + @watcher.interrupt + end + + stdout, = run_watcher(extra_args: ["--fail-fast"], in: fixtures_path.join("example_project")) + + assert_includes(stdout, "[SYSTEM] mt --fail-fast -- test/example_test.rb\n") + end + + def test_watcher_prints_a_status_message_after_successful_test_run + system_proc do |*args| + puts "[SYSTEM] #{args.join(' ')}" + true + end + listen_thread do |callback| + callback.call(["test/example_test.rb"], [], []) + @watcher.interrupt + end + + stdout, = run_watcher(in: fixtures_path.join("example_project")) + + assert_includes(stdout, <<~EXPECTED) + [SYSTEM] mt -- test/example_test.rb + Watching for changes to source and test files. Press ctrl-c to exit. + EXPECTED + end + + def test_watcher_prints_a_status_message_after_failed_test_run + system_proc do |*args| + puts "[SYSTEM] #{args.join(' ')}" + false + end + listen_thread do |callback| + callback.call(["test/example_test.rb"], [], []) + @watcher.interrupt + end + + stdout, = run_watcher(in: fixtures_path.join("example_project")) + + assert_includes(stdout, <<~EXPECTED) + [SYSTEM] mt -- test/example_test.rb + Watching for changes to source and test files. Press ctrl-c to exit. + EXPECTED + end + + def test_watcher_does_nothing_if_a_detected_change_has_no_corresponding_test_file + system_proc { |*args| puts "[SYSTEM] #{args.join(' ')}" } + listen_thread do |callback| + callback.call(["lib/example/version.rb"], [], []) + @watcher.interrupt + end + + stdout, = run_watcher(in: fixtures_path.join("example_project")) + + refute_includes(stdout, "[SYSTEM]") + end + + def test_watcher_restarts_the_listener_when_a_test_run_is_interrupted + thread_count = 0 + system_proc { |*| raise Interrupt } + listen_thread do |callback| + thread_count += 1 + callback.call(["test/example_test.rb"], [], []) + @watcher.interrupt if thread_count > 1 + end + + run_watcher(in: fixtures_path.join("example_project")) + + assert_equal(2, thread_count) + end + + private + + class ListenThread + def initialize(thread, callback) + Thread.new do + thread.call(callback) + end + end + + def start + end + + def stop + end + + def pause + end + + def stopped? + false + end + + def paused? + false + end + end + + def run_watcher(in: ".", extra_args: []) + listen_thread = @listen_thread + file_system = FileSystem.new + file_system.define_singleton_method(:listen) { |&callback| ListenThread.new(listen_thread, callback) } + capture_io do + Dir.chdir(binding.local_variable_get(:in)) do + @watcher = Watcher.new(extra_args:, file_system:, system_proc: @system_proc) + @watcher.run + end + end + end + + def listen_thread(&thread) + @listen_thread = thread + end + + def system_proc(&proc) + @system_proc = proc + end + end +end From 08c1facd5cb0e4cbb663514a5edb455831a13350 Mon Sep 17 00:00:00 2001 From: Matt Brictson Date: Sun, 18 Feb 2024 15:06:16 -0800 Subject: [PATCH 3/4] Rewrite using pattern matching --- lib/mighty_test/watcher.rb | 71 ++++++++++++++------------------ test/mighty_test/watcher_test.rb | 49 ++++++++++------------ 2 files changed, 53 insertions(+), 67 deletions(-) diff --git a/lib/mighty_test/watcher.rb b/lib/mighty_test/watcher.rb index 2a825b9..fb1cf27 100644 --- a/lib/mighty_test/watcher.rb +++ b/lib/mighty_test/watcher.rb @@ -5,71 +5,64 @@ class Watcher WATCHING_FOR_CHANGES = "Watching for changes to source and test files. Press ctrl-c to exit.".freeze def initialize(extra_args: [], file_system: FileSystem.new, system_proc: method(:system)) - @dispatcher = Concurrent::MVar.new + @event = Concurrent::MVar.new @extra_args = extra_args @file_system = file_system @system_proc = system_proc end - def run + def run(iterations: :indefinitely) start_listener puts WATCHING_FOR_CHANGES - process_dispatched_events + + loop = iterations == :indefinitely ? method(:loop) : iterations.method(:times) + loop.call do + case await_next_event + in [:file_system_changed, paths] + mt(*paths) if paths.any? + in [:tests_completed, :pass | :fail] + puts WATCHING_FOR_CHANGES + end + end ensure listener&.stop end - # Normally `run` proceeds indefinitely until the user interrupts with ctrl-c. - # This is a way to stop it gracefully in unit tests. - def interrupt - dispatch { :stop } - end - private attr_reader :extra_args, :file_system, :listener, :system_proc - def file_system_changed(modified, added, _removed) - test_paths = [*modified, *added].filter_map do |path| - file_system.find_matching_test_file(path) - end - return if test_paths.empty? - - run_tests(*test_paths.uniq) - end - - def run_tests(*test_paths) - begin - system_proc.call("mt", *extra_args, "--", *test_paths.flatten) - rescue Interrupt - # Pressing ctrl-c kills the fs_event background process, so we have to manually restart it. - restart_listener - end - puts WATCHING_FOR_CHANGES + def mt(*test_paths) + success = system_proc.call("mt", *extra_args, "--", *test_paths.flatten) + post_event(:tests_completed, success ? :pass : :fail) + rescue Interrupt + # Pressing ctrl-c kills the fs_event background process, so we have to manually restart it. + restart_listener end def start_listener listener.stop if listener && !listener.stopped? - @listener = file_system.listen do |*args| - listener.pause - dispatch do - file_system_changed(*args) - listener.start if listener.paused? + + @listener = file_system.listen do |modified, added, _removed| + # Pause listener so that subsequent changes are queued up while we are running the tests + listener.pause unless listener.stopped? + + test_paths = [*modified, *added].filter_map do |path| + file_system.find_matching_test_file(path) end + + post_event(:file_system_changed, test_paths.uniq) end end alias restart_listener start_listener - def dispatch(&block) - @dispatcher.put(block) - true + def await_next_event + listener.start if listener.paused? + @event.take end - def process_dispatched_events - loop do - result = @dispatcher.take.call - break if result == :stop - end + def post_event(*event) + @event.put(event) end end end diff --git a/test/mighty_test/watcher_test.rb b/test/mighty_test/watcher_test.rb index 58df3b2..bf4c41c 100644 --- a/test/mighty_test/watcher_test.rb +++ b/test/mighty_test/watcher_test.rb @@ -8,22 +8,31 @@ def test_watcher_passes_unique_set_of_test_files_to_mt_command_based_on_changes_ system_proc { |*args| puts "[SYSTEM] #{args.join(' ')}" } listen_thread do |callback| callback.call(["lib/example.rb", "test/focused_test.rb"], ["test/focused_test.rb"], []) - @watcher.interrupt end - stdout, = run_watcher(in: fixtures_path.join("example_project")) + stdout, = run_watcher(iterations: 1, in: fixtures_path.join("example_project")) assert_includes(stdout, "[SYSTEM] mt -- test/example_test.rb test/focused_test.rb\n") end + def test_watcher_does_nothing_if_a_detected_change_has_no_corresponding_test_file + system_proc { |*args| puts "[SYSTEM] #{args.join(' ')}" } + listen_thread do |callback| + callback.call(["lib/example/version.rb"], [], []) + end + + stdout, = run_watcher(iterations: 1, in: fixtures_path.join("example_project")) + + refute_includes(stdout, "[SYSTEM]") + end + def test_watcher_passes_extra_args_through_to_mt_command system_proc { |*args| puts "[SYSTEM] #{args.join(' ')}" } listen_thread do |callback| callback.call(["test/example_test.rb"], [], []) - @watcher.interrupt end - stdout, = run_watcher(extra_args: ["--fail-fast"], in: fixtures_path.join("example_project")) + stdout, = run_watcher(iterations: 1, extra_args: ["--fail-fast"], in: fixtures_path.join("example_project")) assert_includes(stdout, "[SYSTEM] mt --fail-fast -- test/example_test.rb\n") end @@ -35,10 +44,9 @@ def test_watcher_prints_a_status_message_after_successful_test_run end listen_thread do |callback| callback.call(["test/example_test.rb"], [], []) - @watcher.interrupt end - stdout, = run_watcher(in: fixtures_path.join("example_project")) + stdout, = run_watcher(iterations: 2, in: fixtures_path.join("example_project")) assert_includes(stdout, <<~EXPECTED) [SYSTEM] mt -- test/example_test.rb @@ -53,10 +61,9 @@ def test_watcher_prints_a_status_message_after_failed_test_run end listen_thread do |callback| callback.call(["test/example_test.rb"], [], []) - @watcher.interrupt end - stdout, = run_watcher(in: fixtures_path.join("example_project")) + stdout, = run_watcher(iterations: 2, in: fixtures_path.join("example_project")) assert_includes(stdout, <<~EXPECTED) [SYSTEM] mt -- test/example_test.rb @@ -64,35 +71,21 @@ def test_watcher_prints_a_status_message_after_failed_test_run EXPECTED end - def test_watcher_does_nothing_if_a_detected_change_has_no_corresponding_test_file - system_proc { |*args| puts "[SYSTEM] #{args.join(' ')}" } - listen_thread do |callback| - callback.call(["lib/example/version.rb"], [], []) - @watcher.interrupt - end - - stdout, = run_watcher(in: fixtures_path.join("example_project")) - - refute_includes(stdout, "[SYSTEM]") - end - def test_watcher_restarts_the_listener_when_a_test_run_is_interrupted thread_count = 0 system_proc { |*| raise Interrupt } listen_thread do |callback| thread_count += 1 - callback.call(["test/example_test.rb"], [], []) - @watcher.interrupt if thread_count > 1 + callback.call(["test/example_test.rb"], [], []) unless thread_count > 2 end - run_watcher(in: fixtures_path.join("example_project")) - + run_watcher(iterations: 2, in: fixtures_path.join("example_project")) assert_equal(2, thread_count) end private - class ListenThread + class Listener def initialize(thread, callback) Thread.new do thread.call(callback) @@ -117,14 +110,14 @@ def paused? end end - def run_watcher(in: ".", extra_args: []) + def run_watcher(iterations:, in: ".", extra_args: []) listen_thread = @listen_thread file_system = FileSystem.new - file_system.define_singleton_method(:listen) { |&callback| ListenThread.new(listen_thread, callback) } + file_system.define_singleton_method(:listen) { |&callback| Listener.new(listen_thread, callback) } capture_io do Dir.chdir(binding.local_variable_get(:in)) do @watcher = Watcher.new(extra_args:, file_system:, system_proc: @system_proc) - @watcher.run + @watcher.run(iterations:) end end end From d35f93b36a920328b6e0b550e9fb29483e022df5 Mon Sep 17 00:00:00 2001 From: Matt Brictson Date: Sun, 18 Feb 2024 15:10:14 -0800 Subject: [PATCH 4/4] Extract loop logic --- lib/mighty_test/watcher.rb | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/mighty_test/watcher.rb b/lib/mighty_test/watcher.rb index fb1cf27..16c4827 100644 --- a/lib/mighty_test/watcher.rb +++ b/lib/mighty_test/watcher.rb @@ -15,8 +15,7 @@ def run(iterations: :indefinitely) start_listener puts WATCHING_FOR_CHANGES - loop = iterations == :indefinitely ? method(:loop) : iterations.method(:times) - loop.call do + loop_for(iterations) do case await_next_event in [:file_system_changed, paths] mt(*paths) if paths.any? @@ -56,6 +55,10 @@ def start_listener end alias restart_listener start_listener + def loop_for(iterations, &) + iterations == :indefinitely ? loop(&) : iterations.times(&) + end + def await_next_event listener.start if listener.paused? @event.take