diff --git a/.tmp/sv/.gitkeep b/.tmp/sv/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/lib/shopify-cli/helpers/pid_file.rb b/lib/shopify-cli/helpers/pid_file.rb new file mode 100644 index 0000000000..1450a8b966 --- /dev/null +++ b/lib/shopify-cli/helpers/pid_file.rb @@ -0,0 +1,61 @@ +require 'fileutils' + +module ShopifyCli + module Helpers + class PidFile + class << self + RUN_DIR = File.join(ShopifyCli::TEMP_DIR, 'sv') + + def for(identifier) + pid, time = File.read(pid_path_for(identifier)).split(':') + new(identifier, pid: Integer(pid), time: time) + rescue Errno::ENOENT + nil + end + + def write(pid_file) + FileUtils.mkdir_p(File.dirname(pid_file.pid_path)) + File.write(pid_file.pid_path, "#{pid_file.pid}:#{pid_file.time}") + end + + def pid_path_for(identifier) + File.join(RUN_DIR, "#{identifier}.pid") + end + + def log_path_for(identifier) + File.join(RUN_DIR, "#{identifier}.log") + end + end + + attr_reader :identifier, :pid, :time + + def initialize(identifier, pid:, time: Time.now.strftime('%s')) + @identifier = identifier + @pid = pid + @time = time + end + + def pid_path + @pid_path ||= PidFile.pid_path_for(@identifier) + end + + def log_path + @log_path ||= PidFile.log_path_for(@identifier) + end + + def unlink_log + File.unlink(log_path) + nil + rescue Errno::ENOENT + nil + end + + def unlink + File.unlink(pid_path) + nil + rescue Errno::ENOENT + nil + end + end + end +end diff --git a/lib/shopify-cli/helpers/process_supervision.rb b/lib/shopify-cli/helpers/process_supervision.rb index 6fe5696e44..be4a90cd43 100644 --- a/lib/shopify-cli/helpers/process_supervision.rb +++ b/lib/shopify-cli/helpers/process_supervision.rb @@ -6,17 +6,26 @@ module ProcessSupervision DebriefableError = Class.new(StandardError) class << self - def start(args) - pid = fork do + def start(identifier, args) + fork do + pid_file = PidFile.new(identifier, pid: Process.pid) + PidFile.write(pid_file) + + STDOUT.reopen(pid_file.log_path, "w") + STDERR.reopen(pid_file.log_path, "w") STDIN.reopen("/dev/null", "r") Process.setsid exec(*args) end - pid + sleep(0.1) end - def stop(pid) + def stop(identifier) + pid_file = pid_file_for_id(identifier) + return unless pid_file + pid = pid_file.pid + process_group = -pid stop_pid(process_group) rescue Errno::ESRCH @@ -28,8 +37,10 @@ def stop(pid) end end - def running?(pid) - pid_alive?(pid) + def running?(identifier) + pid_file = PidFile.for(identifier) + return false unless pid_file + pid_alive?(pid_file.pid) end private @@ -51,6 +62,17 @@ def pid_alive?(pid) rescue Errno::EPERM true end + + def pid_file_for_id(identifier) + pid_file = PidFile.for(identifier) + return nil unless pid_file + return pid_file if pid_alive?(pid_file.pid) + + pid_file.unlink # clean up if pidfile specifies dead pid + nil + rescue Errno::ENOENT + nil + end end end end diff --git a/lib/shopify-cli/tasks/tunnel.rb b/lib/shopify-cli/tasks/tunnel.rb index 44e81c683a..37cdd4cdb6 100644 --- a/lib/shopify-cli/tasks/tunnel.rb +++ b/lib/shopify-cli/tasks/tunnel.rb @@ -22,8 +22,9 @@ def call(ctx) def stop(ctx) @ctx = ctx if running? - ShopifyCli::Helpers::ProcessSupervision.stop(state[:pid]) - FileUtils.rm(pid_file) + ShopifyCli::Helpers::ProcessSupervision.stop(:ngrok) + pid_file = ShopifyCli::Helpers::PidFile.for(:ngrok) + pid_file&.unlink_log @ctx.puts("{{green:x}} ngrok tunnel stopped") else @ctx.puts("{{green:x}} ngrok tunnel not running") @@ -34,39 +35,21 @@ def start install unless running? - pid = ShopifyCli::Helpers::ProcessSupervision.start(ngrok_command) - write_state(pid, Time.now) + ShopifyCli::Helpers::ProcessSupervision.start(:ngrok, ngrok_command) end - url = fetch_url + pid_file = ShopifyCli::Helpers::PidFile.for(:ngrok) + url = fetch_url(pid_file.log_path) @ctx.puts("{{green:✔︎}} ngrok tunnel running at #{url}") @ctx.app_metadata = { host: url } url end def running? - return false unless File.exist?(pid_file) - ShopifyCli::Helpers::ProcessSupervision.running?(state[:pid]) + ShopifyCli::Helpers::ProcessSupervision.running?(:ngrok) end private - def read_state - content = JSON.parse(File.open(pid_file).read) - { - pid: content['pid'], - time: content['time'], - } - end - - def write_state(pid, time) - File.open(pid_file, 'w') do |f| - f.write({ - pid: pid, - time: time, - }.to_json) - end - end - def install return if File.exist?(File.join(ShopifyCli::ROOT, 'ngrok')) spinner = CLI::UI::SpinGroup.new @@ -81,10 +64,10 @@ def install spinner.wait end - def fetch_url + def fetch_url(log_path) counter = 0 while counter < TIMEOUT - log_content = log.read + log_content = File.read(log_path) result = log_content.match(/msg="started tunnel".*url=(https:\/\/.+)/) return result[1] if result @@ -102,34 +85,7 @@ def fetch_url end def ngrok_command - "exec #{File.join(ShopifyCli::ROOT, 'ngrok')} http -log=stdout -log-level=debug #{PORT} > #{log}" - end - - def log - @log ||= Logger.new - end - - def pid_file - File.join(ShopifyCli::ROOT, '.tmp/ngrok.pid') - end - - def state - @state ||= read_state - end - - class Logger - def initialize(location = File.join(ShopifyCli::ROOT, '.tmp', 'ngrok.log')) - FileUtils.mkdir_p(File.dirname(location)) - @location = location - end - - def read - File.read(@location) - end - - def to_s - @location - end + "exec #{File.join(ShopifyCli::ROOT, 'ngrok')} http -log=stdout -log-level=debug #{PORT}" end end end diff --git a/lib/shopify_cli.rb b/lib/shopify_cli.rb index 2275893120..da5ba7df35 100644 --- a/lib/shopify_cli.rb +++ b/lib/shopify_cli.rb @@ -47,6 +47,7 @@ module ShopifyCli TOOL_NAME = 'shopify' ROOT = File.expand_path('../..', __FILE__) INSTALL_DIR = File.expand_path(File.join(ENV.fetch('XDG_RUNTIME_DIR', ENV.fetch('HOME')), '.shopify-cli')) + TEMP_DIR = File.join(ROOT, '.tmp') CONFIG_HOME = File.expand_path(ENV.fetch('XDG_CONFIG_HOME', '~/.config')) TOOL_CONFIG_PATH = File.join(CONFIG_HOME, TOOL_NAME) LOG_FILE = File.join(TOOL_CONFIG_PATH, 'logs', 'log.log') @@ -114,6 +115,7 @@ module Tasks end module Helpers + autoload :PidFile, 'shopify-cli/helpers/pid_file' autoload :GemHelper, 'shopify-cli/helpers/gem_helper' autoload :EnvFileHelper, 'shopify-cli/helpers/env_file_helper' autoload :ProcessSupervision, 'shopify-cli/helpers/process_supervision' diff --git a/test/shopify-cli/helpers/pid_file_test.rb b/test/shopify-cli/helpers/pid_file_test.rb new file mode 100644 index 0000000000..6d4514b538 --- /dev/null +++ b/test/shopify-cli/helpers/pid_file_test.rb @@ -0,0 +1,22 @@ +require 'test_helper' + +module ShopifyCli + module Helpers + class PidFileTest < MiniTest::Test + def setup + @pid_file = PidFile.new('web', pid: 1234) + end + + def test_pid_has_expected_attributes + assert_equal(1234, @pid_file.pid) + assert_equal('web', @pid_file.identifier) + assert_equal( + File.join(ShopifyCli::TEMP_DIR, 'sv/web.pid'), @pid_file.pid_path + ) + assert_equal( + File.join(ShopifyCli::TEMP_DIR, 'sv/web.log'), @pid_file.log_path + ) + end + end + end +end diff --git a/test/shopify-cli/helpers/process_supervision_test.rb b/test/shopify-cli/helpers/process_supervision_test.rb index b38c7751b6..a7dd35f5cb 100644 --- a/test/shopify-cli/helpers/process_supervision_test.rb +++ b/test/shopify-cli/helpers/process_supervision_test.rb @@ -4,30 +4,39 @@ module ShopifyCli module Helpers class ProcessSuperVisionTest < MiniTest::Test def teardown - Process.kill("TERM", @pid) - rescue Errno::EPERM - # Happens with start_fg due to exec-ing the forked process - rescue Errno::ESRCH - # It's okay if it's not running anymore + return unless @pid_file + + begin + Process.kill("TERM", @pid_file.pid) + rescue Errno::EPERM + # Happens with start_fg due to exec-ing the forked process + rescue Errno::ESRCH + # It's okay if it's not running anymore + end + + @pid_file.unlink + @pid_file.unlink_log end def test_start - @pid = ProcessSupervision.start("sleep 1") + ProcessSupervision.start('example', 'sleep 1') + @pid_file = PidFile.for('example') - assert_process_running(@pid) + assert_process_running(@pid_file.pid) - assert ProcessSupervision.running?(@pid) + assert ProcessSupervision.running?('example') end def test_stop - @pid = spawn_fake_process("should_stop_me") - assert_process_running(@pid) + spawn_fake_process('example') + @pid_file = PidFile.for('example') + assert_process_running(@pid_file.pid) - ProcessSupervision.stop(@pid) + ProcessSupervision.stop('example') - refute_process_running(@pid) + refute_process_running(@pid_file.pid) - refute ProcessSupervision.running?(@pid) + refute ProcessSupervision.running?('example') end private @@ -55,7 +64,7 @@ def process_running?(pid) false end - def spawn_fake_process(name) + def spawn_fake_process(identifier) reader, writer = IO.pipe pid = fork do @@ -78,7 +87,9 @@ def spawn_fake_process(name) writer.close reader.read - pid + PidFile.new(identifier, pid: pid).tap do |pid_file| + PidFile.write(pid_file) + end end end end diff --git a/test/task/tunnel_test.rb b/test/task/tunnel_test.rb index 8f72780b86..2df2037e3f 100644 --- a/test/task/tunnel_test.rb +++ b/test/task/tunnel_test.rb @@ -6,32 +6,30 @@ class TunnelTest < MiniTest::Test include TestHelpers::Context def setup - Tunnel.any_instance.stubs(:pid_file).returns(pid_path) Tunnel.any_instance.stubs(:install) super end def test_start_running_returns_url + ShopifyCli::Helpers::ProcessSupervision.stubs(:running?) + .with(:ngrok).returns(:true) with_log do - ShopifyCli::Helpers::ProcessSupervision.stubs(:running?) - .with(40000).returns(:true) - FakeFS::FileSystem.clone(pid_path) Tunnel.new.call(@context) assert_equal 'https://example.ngrok.io', @context.app_metadata[:host] end end def test_start_not_running_starts_ngrok - ShopifyCli::Helpers::ProcessSupervision.stubs(:running?).returns(false) with_log do + ShopifyCli::Helpers::ProcessSupervision.stubs(:running?).returns(false) ShopifyCli::Helpers::ProcessSupervision.expects(:start).with( - "exec #{File.join(ShopifyCli::ROOT, 'ngrok')} http -log=stdout -log-level=debug 8081 > /tmp/ngrok.log" - ).returns(40004) + :ngrok, + "exec #{File.join(ShopifyCli::ROOT, 'ngrok')} http -log=stdout -log-level=debug 8081" + ) @context.expects(:puts).with( "{{green:✔︎}} ngrok tunnel running at https://example.ngrok.io" ) assert_equal 'https://example.ngrok.io', ShopifyCli::Tasks::Tunnel.new.call(@context) - assert File.read(pid_path).include?('"pid":40004') assert_equal 'https://example.ngrok.io', @context.app_metadata[:host] end end @@ -40,7 +38,8 @@ def test_start_raises_error_on_ngrok_failure Tunnel.any_instance.stubs(:running?).returns(false) with_log('ngrok_error') do ShopifyCli::Helpers::ProcessSupervision.expects(:start).with( - "exec #{File.join(ShopifyCli::ROOT, 'ngrok')} http -log=stdout -log-level=debug 8081 > /tmp/ngrok.log" + :ngrok, + "exec #{File.join(ShopifyCli::ROOT, 'ngrok')} http -log=stdout -log-level=debug 8081" ) assert_raises ShopifyCli::Tasks::Tunnel::NgrokError do Tunnel.new.call(@context) @@ -48,23 +47,21 @@ def test_start_raises_error_on_ngrok_failure end end - def with_log(fixture = 'ngrok') - log_path = File.join(ShopifyCli::ROOT, "test/fixtures/#{fixture}.log") - FakeFS::FileSystem.clone(log_path) - Tunnel.any_instance.stubs(:log).returns( - FakeLogger.new(log_path) - ) - yield(log_path) - end - - def pid_path - @pid_path ||= File.join(ShopifyCli::ROOT, 'test/fixtures/ngrok.pid') + def test_stop_removes_pid_and_logfile + Tunnel.any_instance.stubs(:running?).returns(true) + @context.expects(:puts).with("{{green:x}} ngrok tunnel stopped") + ShopifyCli::Helpers::ProcessSupervision.expects(:stop).with(:ngrok) + Tunnel.new.stop(@context) end - class FakeLogger < ShopifyCli::Tasks::Tunnel::Logger - def to_s - '/tmp/ngrok.log' - end + def with_log(fixture = 'ngrok') + log_path = File.join(ShopifyCli::ROOT, "test/fixtures/#{fixture}.log") + pid_file = ShopifyCli::Helpers::PidFile.new(:ngrok, pid: 40000) + ShopifyCli::Helpers::PidFile.write(pid_file) + File.write(pid_file.log_path, File.read(log_path)) + yield + pid_file.unlink + pid_file.unlink_log end end end diff --git a/test/test_helpers/constants.rb b/test/test_helpers/constants.rb index d34aca20bd..5117cbddfe 100644 --- a/test/test_helpers/constants.rb +++ b/test/test_helpers/constants.rb @@ -2,6 +2,11 @@ module TestHelpers module Constants protected + def teardown + super + reset_constants + end + def redefine_constant(mod, constant, new_value) @redefined_constants ||= [] @redefined_constants << [mod, constant, mod.const_get(constant)] diff --git a/test/test_helpers/context.rb b/test/test_helpers/context.rb index 7e2a01215f..cca8953552 100644 --- a/test/test_helpers/context.rb +++ b/test/test_helpers/context.rb @@ -1,12 +1,9 @@ # frozen_string_literal: true module TestHelpers module Context - include TestHelpers::FakeFS - def setup @context = TestHelpers::FakeContext.new @context.root = Dir.mktmpdir - ::FakeFS::FileSystem.clone(@context.root) FileUtils.touch(File.join(@context.root, '.shopify-cli.yml')) super FileUtils.cd(@context.root)