From 0088fe3deaa97568b79f2bda4fc3cc3b6b442316 Mon Sep 17 00:00:00 2001 From: schneems Date: Tue, 2 Nov 2021 10:14:18 -0500 Subject: [PATCH 1/5] Faster command line spec This is the slowest test in the suite, taking longer than 1 second. I had the logic duplicated for some weird reason, removing that got me twice as fast. Since we need to boot 2 processes (the slow process), we can spawn them both before waiting on the first one to finish. --- spec/integration/ruby_command_line_spec.rb | 23 +++++++++++----------- 1 file changed, 11 insertions(+), 12 deletions(-) diff --git a/spec/integration/ruby_command_line_spec.rb b/spec/integration/ruby_command_line_spec.rb index f79a048..ce6912a 100644 --- a/spec/integration/ruby_command_line_spec.rb +++ b/spec/integration/ruby_command_line_spec.rb @@ -6,23 +6,22 @@ module DeadEnd RSpec.describe "Requires with ruby cli" do it "namespaces all monkeypatched methods" do Dir.mktmpdir do |dir| - @tmpdir = Pathname(dir) - @script = @tmpdir.join("script.rb") - @script.write <<~'EOM' + tmpdir = Pathname(dir) + script = tmpdir.join("script.rb") + script.write <<~'EOM' puts Kernel.private_methods EOM + dead_end_methods_file = tmpdir.join("dead_end_methods.txt") + kernel_methods_file = tmpdir.join("kernel_methods.txt") - dead_end_methods_array = `ruby -I#{lib_dir} -rdead_end/auto #{@script} 2>&1`.strip.lines.map(&:strip) - kernel_methods_array = `ruby #{@script} 2>&1`.strip.lines.map(&:strip) - methods = (dead_end_methods_array - kernel_methods_array).sort - expect(methods).to eq(["dead_end_original_load", "dead_end_original_require", "dead_end_original_require_relative", "timeout"]) + d_pid = Process.spawn("ruby -I#{lib_dir} -rdead_end/auto #{script} 2>&1 > #{dead_end_methods_file}") + k_pid = Process.spawn("ruby #{script} 2>&1 >> #{kernel_methods_file}") - @script.write <<~'EOM' - puts Kernel.private_methods - EOM + Process.wait(k_pid) + Process.wait(d_pid) - dead_end_methods_array = `ruby -I#{lib_dir} -rdead_end/auto #{@script} 2>&1`.strip.lines.map(&:strip) - kernel_methods_array = `ruby #{@script} 2>&1`.strip.lines.map(&:strip) + dead_end_methods_array = dead_end_methods_file.read.strip.lines.map(&:strip) + kernel_methods_array = kernel_methods_file.read.strip.lines.map(&:strip) methods = (dead_end_methods_array - kernel_methods_array).sort expect(methods).to eq(["dead_end_original_load", "dead_end_original_require", "dead_end_original_require_relative", "timeout"]) end From 9fbdf3d31672a616dee96d6d5bb36df8d5967b35 Mon Sep 17 00:00:00 2001 From: schneems Date: Tue, 2 Nov 2021 10:14:38 -0500 Subject: [PATCH 2/5] Move instance variables to local variables Total test time is now 3.78 --- spec/integration/ruby_command_line_spec.rb | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/spec/integration/ruby_command_line_spec.rb b/spec/integration/ruby_command_line_spec.rb index ce6912a..eed66ef 100644 --- a/spec/integration/ruby_command_line_spec.rb +++ b/spec/integration/ruby_command_line_spec.rb @@ -29,9 +29,9 @@ module DeadEnd it "detects require error and adds a message with auto mode" do Dir.mktmpdir do |dir| - @tmpdir = Pathname(dir) - @script = @tmpdir.join("script.rb") - @script.write <<~EOM + tmpdir = Pathname(dir) + script = tmpdir.join("script.rb") + script.write <<~EOM describe "things" do it "blerg" do end @@ -44,14 +44,15 @@ module DeadEnd end EOM - require_rb = @tmpdir.join("require.rb") + require_rb = tmpdir.join("require.rb") require_rb.write <<~EOM require_relative "./script.rb" EOM - `ruby -I#{lib_dir} -rdead_end #{require_rb} 2>&1` + out = `ruby -I#{lib_dir} -rdead_end #{require_rb} 2>&1` expect($?.success?).to be_falsey + expect(out).to include('❯ 5 it "flerg"') end end end From 426b5483920013640aaf43fb29881f8d2f8b8a02 Mon Sep 17 00:00:00 2001 From: schneems Date: Tue, 2 Nov 2021 10:19:28 -0500 Subject: [PATCH 3/5] Move EXE test to dead_end Booting processes is slow. Total test time is now 3.37 --- spec/integration/dead_end_spec.rb | 21 +++++++++++++++++++++ spec/integration/exe_cli_spec.rb | 20 -------------------- 2 files changed, 21 insertions(+), 20 deletions(-) diff --git a/spec/integration/dead_end_spec.rb b/spec/integration/dead_end_spec.rb index 47174a6..de52dde 100644 --- a/spec/integration/dead_end_spec.rb +++ b/spec/integration/dead_end_spec.rb @@ -92,5 +92,26 @@ module DeadEnd debug_display(io.string) debug_display(benchmark) end + + it "handles heredocs" do + lines = fixtures_dir.join("rexe.rb.txt").read.lines + lines.delete_at(85 - 1) + io = StringIO.new + DeadEnd.call( + io: io, + source: lines.join, + ) + + out = io.string + debug_display(out) + + expect(out).to include(<<~EOM) + 16 class Rexe + ❯ 77 class Lookups + ❯ 78 def input_modes + ❯ 148 end + 551 end + EOM + end end end diff --git a/spec/integration/exe_cli_spec.rb b/spec/integration/exe_cli_spec.rb index 2696a5f..59f4987 100644 --- a/spec/integration/exe_cli_spec.rb +++ b/spec/integration/exe_cli_spec.rb @@ -30,26 +30,6 @@ def exe(cmd) expect($?.success?).to be_falsey end - it "handles heredocs" do - lines = fixtures_dir.join("rexe.rb.txt").read.lines - Tempfile.create do |file| - lines.delete_at(85 - 1) - - Pathname(file.path).write(lines.join) - - out = exe(file.path) - debug_display(out) - - expect(out).to include(<<~EOM) - 16 class Rexe - ❯ 77 class Lookups - ❯ 78 def input_modes - ❯ 148 end - 551 end - EOM - end - end - # When ruby sub shells it is not a interactive shell and dead_end will # default to no coloring. Colors/bold can be forced with `--terminal` # flag From 7585255b0c7ad068216c7dbb36dfcb260f0c8b00 Mon Sep 17 00:00:00 2001 From: schneems Date: Tue, 2 Nov 2021 16:05:58 -0500 Subject: [PATCH 4/5] Move CLI logic to dedicated class Shelling out to the executable is very expensive. Moving this logic into the class means we can unit test it which is much faster. Total test time is now 1.72 seconds --- exe/dead_end | 80 +------------- lib/dead_end.rb | 1 + lib/dead_end/cli.rb | 119 +++++++++++++++++++++ spec/integration/exe_cli_spec.rb | 41 -------- spec/unit/cli_spec.rb | 175 +++++++++++++++++++++++++++++++ 5 files changed, 298 insertions(+), 118 deletions(-) create mode 100644 lib/dead_end/cli.rb create mode 100644 spec/unit/cli_spec.rb diff --git a/exe/dead_end b/exe/dead_end index e83ac53..9ae3301 100755 --- a/exe/dead_end +++ b/exe/dead_end @@ -1,81 +1,7 @@ #!/usr/bin/env ruby -require "pathname" -require "optparse" require_relative "../lib/dead_end" -options = {} -options[:record_dir] = ENV["DEAD_END_RECORD_DIR"] - -parser = OptionParser.new do |opts| - opts.banner = <<~EOM - Usage: dead_end [options] - - Parses a ruby source file and searches for syntax error(s) such as - unexpected `end', expecting end-of-input. - - Example: - - $ dead_end dog.rb - - # ... - - ❯ 10 defdog - ❯ 15 end - ❯ 16 - - Env options: - - DEAD_END_RECORD_DIR= - - When enabled, records the steps used to search for a syntax error to the - given directory - - Options: - EOM - - opts.version = DeadEnd::VERSION - - opts.on("--help", "Help - displays this message") do |v| - puts opts - exit - end - - opts.on("--record ", "When enabled, records the steps used to search for a syntax error to the given directory") do |v| - options[:record_dir] = v - end - - opts.on("--terminal", "Enable terminal highlighting") do |v| - options[:terminal] = true - end - - opts.on("--no-terminal", "Disable terminal highlighting") do |v| - options[:terminal] = false - end -end -parser.parse! - -file = ARGV[0] - -if file.nil? || file.empty? - # Display help if raw command - parser.parse! %w[--help] -end - -file = Pathname(file) -options[:record_dir] = "tmp" if ENV["DEBUG"] - -warn "Record dir: #{options[:record_dir]}" if options[:record_dir] - -display = DeadEnd.call( - source: file.read, - filename: file.expand_path, - terminal: options.fetch(:terminal, DeadEnd::DEFAULT_VALUE), - record_dir: options[:record_dir] -) - -if display.document_ok? - exit(0) -else - exit(1) -end +DeadEnd::Cli.new( + argv: ARGV +).call diff --git a/lib/dead_end.rb b/lib/dead_end.rb index e8b285b..900f6e5 100644 --- a/lib/dead_end.rb +++ b/lib/dead_end.rb @@ -145,3 +145,4 @@ def self.valid?(source) require_relative "dead_end/explain_syntax" require_relative "dead_end/auto" +require_relative "dead_end/cli" diff --git a/lib/dead_end/cli.rb b/lib/dead_end/cli.rb new file mode 100644 index 0000000..3cedcdb --- /dev/null +++ b/lib/dead_end/cli.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +require "pathname" +require "optparse" + +module DeadEnd + # All the logic of the exe/dead_end CLI in one handy spot + # + # Cli.new(argv: ["--help"]).call + # Cli.new(argv: [".rb"]).call + # Cli.new(argv: [".rb", "--record=tmp"]).call + # Cli.new(argv: [".rb", "--terminal"]).call + # + class Cli + attr_accessor :options, :file_name + + # ARGV is Everything passed to the executable, does not include executable name + # + # All other intputs are dependency injection for testing + def initialize(argv: , exit_obj: Kernel, io: $stdout, env: ENV) + @options = {} + @parser = nil + options[:record_dir] = env["DEAD_END_RECORD_DIR"] + options[:record_dir] = "tmp" if env["DEBUG"] + options[:terminal] = DeadEnd::DEFAULT_VALUE + + @io = io + @argv = argv + @file_name = argv[0] + @exit_obj = exit_obj + end + + def call + if file_name.nil? || file_name.empty? + # Display help if raw command + parser.parse! %w[--help] + else + self.parse + end + + # Needed for testing since we fake exit + return if options[:exit] + + file = Pathname(file_name) + + @io.puts "Record dir: #{options[:record_dir]}" if options[:record_dir] + + display = DeadEnd.call( + io: @io, + source: file.read, + filename: file.expand_path, + terminal: options.fetch(:terminal, DeadEnd::DEFAULT_VALUE), + record_dir: options[:record_dir] + ) + + if display.document_ok? + @exit_obj.exit(0) + else + @exit_obj.exit(1) + end + end + + def parse + parser.parse!(@argv) + + self + end + + def parser + @parser ||= OptionParser.new do |opts| + + opts.banner = <<~EOM + Usage: dead_end [options] + + Parses a ruby source file and searches for syntax error(s) such as + unexpected `end', expecting end-of-input. + + Example: + + $ dead_end dog.rb + + # ... + + ❯ 10 defdog + ❯ 15 end + + ENV options: + + DEAD_END_RECORD_DIR= + + Records the steps used to search for a syntax error + to the given directory + + Options: + EOM + + opts.version = DeadEnd::VERSION + + opts.on("--help", "Help - displays this message") do |v| + @io.puts opts + options[:exit] = true + @exit_obj.exit + end + + opts.on("--record ", "Records the steps used to search for a syntax error to the given directory") do |v| + options[:record_dir] = v + end + + opts.on("--terminal", "Enable terminal highlighting") do |v| + options[:terminal] = true + end + + opts.on("--no-terminal", "Disable terminal highlighting") do |v| + options[:terminal] = false + end + end + end + end +end diff --git a/spec/integration/exe_cli_spec.rb b/spec/integration/exe_cli_spec.rb index 59f4987..5a49d9a 100644 --- a/spec/integration/exe_cli_spec.rb +++ b/spec/integration/exe_cli_spec.rb @@ -14,47 +14,6 @@ def exe(cmd) out end - it "parses valid code" do - ruby_file = exe_path - out = exe(ruby_file) - expect(out.strip).to include("Syntax OK") - expect($?.success?).to be_truthy - end - - it "parses invalid code" do - ruby_file = fixtures_dir.join("this_project_extra_def.rb.txt") - out = exe(ruby_file) - debug_display(out) - - expect(out.strip).to include("❯ 36 def filename") - expect($?.success?).to be_falsey - end - - # When ruby sub shells it is not a interactive shell and dead_end will - # default to no coloring. Colors/bold can be forced with `--terminal` - # flag - it "passing --terminal will force color codes" do - ruby_file = fixtures_dir.join("this_project_extra_def.rb.txt") - out = exe("#{ruby_file} --terminal") - - expect(out.strip).to include("\e[0m❯ 36 \e[1;3m def filename") - end - - it "records search" do - Dir.mktmpdir do |dir| - dir = Pathname(dir) - tmp_dir = dir.join("tmp").tap(&:mkpath) - ruby_file = dir.join("file.rb") - ruby_file.write("def foo\n end\nend") - - expect(tmp_dir).to be_empty - - exe("#{ruby_file} --record #{tmp_dir}") - - expect(tmp_dir).to_not be_empty - end - end - it "prints the version" do out = exe("-v") expect(out.strip).to include(DeadEnd::VERSION) diff --git a/spec/unit/cli_spec.rb b/spec/unit/cli_spec.rb new file mode 100644 index 0000000..4fed36c --- /dev/null +++ b/spec/unit/cli_spec.rb @@ -0,0 +1,175 @@ +# frozen_string_literal: true + +require_relative "../spec_helper" + +module DeadEnd + + class FakeExit + def initialize + @called = false + @value = nil + end + + def exit(value = nil) + @called = true + @value = value + end + + def called? + @called + end + + def value + @value + end + end + + RSpec.describe Cli do + it "parses valid code" do + Dir.mktmpdir do |dir| + dir = Pathname(dir) + file = dir.join("script.rb") + file.write("puts 'lol'") + + io = StringIO.new + exit_obj = FakeExit.new + cli = Cli.new( + io: io, + argv: [file.to_s], + exit_obj: exit_obj + ).call + + expect(exit_obj.called?).to be_truthy + expect(exit_obj.value).to eq(0) + expect(io.string.strip).to eq("Syntax OK") + end + end + + it "parses invalid code" do + file = fixtures_dir.join("this_project_extra_def.rb.txt") + + io = StringIO.new + exit_obj = FakeExit.new + cli = Cli.new( + io: io, + argv: [file.to_s], + exit_obj: exit_obj + ).call + + out = io.string + debug_display(out) + + expect(exit_obj.called?).to be_truthy + expect(exit_obj.value).to eq(1) + expect(out.strip).to include("❯ 36 def filename") + end + + # We cannot execute the parser here + # because it calls `exit` and it will exit + # our tests, however we can assert that the + # parser has the right value for version + it "-v version" do + io = StringIO.new + exit_obj = FakeExit.new + parser = Cli.new( + io: io, + argv: ["-v"], + exit_obj: exit_obj + ).parser + + expect(parser.version).to include(DeadEnd::VERSION.to_s) + end + + it "DEAD_END_RECORD_DIR" do + io = StringIO.new + exit_obj = FakeExit.new + cli = Cli.new( + io: io, + argv: [], + env: {"DEAD_END_RECORD_DIR" => "hahaha"}, + exit_obj: exit_obj + ).parse + + expect(exit_obj.called?).to be_falsey + expect(cli.options[:record_dir]).to eq("hahaha") + end + + it "--record-dir=" do + io = StringIO.new + exit_obj = FakeExit.new + cli = Cli.new( + io: io, + argv: ["--record=lol"], + exit_obj: exit_obj + ).parse + + expect(exit_obj.called?).to be_falsey + expect(cli.options[:record_dir]).to eq("lol") + end + + it "terminal default to respecting TTY" do + io = StringIO.new + exit_obj = FakeExit.new + cli = Cli.new( + io: io, + argv: [], + exit_obj: exit_obj + ).parse + + expect(exit_obj.called?).to be_falsey + expect(cli.options[:terminal]).to eq(DeadEnd::DEFAULT_VALUE) + end + + it "--terminal" do + io = StringIO.new + exit_obj = FakeExit.new + cli = Cli.new( + io: io, + argv: ["--terminal"], + exit_obj: exit_obj + ).parse + + expect(exit_obj.called?).to be_falsey + expect(cli.options[:terminal]).to be_truthy + end + + it "--no-terminal" do + io = StringIO.new + exit_obj = FakeExit.new + cli = Cli.new( + io: io, + argv: ["--no-terminal"], + exit_obj: exit_obj + ).parse + + expect(exit_obj.called?).to be_falsey + expect(cli.options[:terminal]).to be_falsey + end + + it "--help outputs help" do + io = StringIO.new + exit_obj = FakeExit.new + Cli.new( + io: io, + argv: ["--help"], + exit_obj: exit_obj + ).call + + expect(exit_obj.called?).to be_truthy + expect(io.string).to include("Usage: dead_end [options]") + end + + it " outputs help" do + io = StringIO.new + exit_obj = FakeExit.new + Cli.new( + io: io, + argv: [], + exit_obj: exit_obj + ).call + + expect(exit_obj.called?).to be_truthy + expect(io.string).to include("Usage: dead_end [options]") + end + end +end From a37e7f06cc38be0db7530d5d193cd7d08b2ec355 Mon Sep 17 00:00:00 2001 From: schneems Date: Tue, 2 Nov 2021 16:34:38 -0500 Subject: [PATCH 5/5] Standardrb --fix --- lib/dead_end/cli.rb | 5 ++--- spec/integration/dead_end_spec.rb | 2 +- spec/unit/cli_spec.rb | 9 +++------ 3 files changed, 6 insertions(+), 10 deletions(-) diff --git a/lib/dead_end/cli.rb b/lib/dead_end/cli.rb index 3cedcdb..a81d9a3 100644 --- a/lib/dead_end/cli.rb +++ b/lib/dead_end/cli.rb @@ -17,7 +17,7 @@ class Cli # ARGV is Everything passed to the executable, does not include executable name # # All other intputs are dependency injection for testing - def initialize(argv: , exit_obj: Kernel, io: $stdout, env: ENV) + def initialize(argv:, exit_obj: Kernel, io: $stdout, env: ENV) @options = {} @parser = nil options[:record_dir] = env["DEAD_END_RECORD_DIR"] @@ -35,7 +35,7 @@ def call # Display help if raw command parser.parse! %w[--help] else - self.parse + parse end # Needed for testing since we fake exit @@ -68,7 +68,6 @@ def parse def parser @parser ||= OptionParser.new do |opts| - opts.banner = <<~EOM Usage: dead_end [options] diff --git a/spec/integration/dead_end_spec.rb b/spec/integration/dead_end_spec.rb index de52dde..e21e9a8 100644 --- a/spec/integration/dead_end_spec.rb +++ b/spec/integration/dead_end_spec.rb @@ -99,7 +99,7 @@ module DeadEnd io = StringIO.new DeadEnd.call( io: io, - source: lines.join, + source: lines.join ) out = io.string diff --git a/spec/unit/cli_spec.rb b/spec/unit/cli_spec.rb index 4fed36c..79a3340 100644 --- a/spec/unit/cli_spec.rb +++ b/spec/unit/cli_spec.rb @@ -3,7 +3,6 @@ require_relative "../spec_helper" module DeadEnd - class FakeExit def initialize @called = false @@ -19,9 +18,7 @@ def called? @called end - def value - @value - end + attr_reader :value end RSpec.describe Cli do @@ -33,7 +30,7 @@ def value io = StringIO.new exit_obj = FakeExit.new - cli = Cli.new( + Cli.new( io: io, argv: [file.to_s], exit_obj: exit_obj @@ -50,7 +47,7 @@ def value io = StringIO.new exit_obj = FakeExit.new - cli = Cli.new( + Cli.new( io: io, argv: [file.to_s], exit_obj: exit_obj