From 96ea13e0202fd01a28ee393bfb515dfbd61abc61 Mon Sep 17 00:00:00 2001 From: Gannon McGibbon Date: Tue, 19 Nov 2024 15:49:59 -0600 Subject: [PATCH] Add local firefox viewer Adds local firefox viewer and mkes it the default for vernier viewing. --- lib/app_profiler.rb | 3 +- lib/app_profiler/exec.rb | 37 +++++++++ lib/app_profiler/viewer/firefox_viewer.rb | 79 +++++++++++++++++++ lib/app_profiler/yarn/command.rb | 34 ++------ .../yarn/with_firefox_profiler.rb | 12 ++- .../middleware/firefox_middleware_test.rb | 12 +-- .../middleware/speedscope_middleware_test.rb | 11 ++- .../viewer/speedscope_viewer_test.rb | 32 +++++--- test/app_profiler/yarn/command_test.rb | 15 ++-- 9 files changed, 181 insertions(+), 54 deletions(-) create mode 100644 lib/app_profiler/exec.rb create mode 100644 lib/app_profiler/viewer/firefox_viewer.rb diff --git a/lib/app_profiler.rb b/lib/app_profiler.rb index ff79bdb7..8fa484d3 100644 --- a/lib/app_profiler.rb +++ b/lib/app_profiler.rb @@ -30,6 +30,7 @@ module Storage module Viewer autoload :BaseViewer, "app_profiler/viewer/base_viewer" autoload :SpeedscopeViewer, "app_profiler/viewer/speedscope_viewer" + autoload :FirefoxViewer, "app_profiler/viewer/firefox_viewer" autoload :BaseMiddleware, "app_profiler/viewer/base_middleware" autoload :SpeedscopeRemoteViewer, "app_profiler/viewer/speedscope_remote_viewer" autoload :FirefoxRemoteViewer, "app_profiler/viewer/firefox_remote_viewer" @@ -125,7 +126,7 @@ def stackprof_viewer end def vernier_viewer - @@vernier_viewer ||= Viewer::FirefoxRemoteViewer # rubocop:disable Style/ClassVars + @@vernier_viewer ||= Viewer::FirefoxViewer # rubocop:disable Style/ClassVars end def profile_sampler_enabled=(value) diff --git a/lib/app_profiler/exec.rb b/lib/app_profiler/exec.rb new file mode 100644 index 00000000..1932ab25 --- /dev/null +++ b/lib/app_profiler/exec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module AppProfiler + module Exec # :nodoc: + protected + + def valid_commands + raise NotImplementedError + end + + def ensure_command_valid(command) + unless valid_command?(command) + raise ArgumentError, "Illegal command: #{command.join(" ")}." + end + end + + def valid_command?(command) + valid_commands.any? do |valid_command| + next unless valid_command.size == command.size + + valid_command.zip(command).all? do |valid_part, part| + part.match?(valid_part) + end + end + end + + def exec(*command, silent: false, environment: {}) + ensure_command_valid(command) + + if silent + system(environment, *command, out: File::NULL).tap { |return_code| yield unless return_code } + else + system(environment, *command).tap { |return_code| yield unless return_code } + end + end + end +end diff --git a/lib/app_profiler/viewer/firefox_viewer.rb b/lib/app_profiler/viewer/firefox_viewer.rb new file mode 100644 index 00000000..bf274920 --- /dev/null +++ b/lib/app_profiler/viewer/firefox_viewer.rb @@ -0,0 +1,79 @@ +# frozen_string_literal: true + +require "app_profiler/exec" + +module AppProfiler + module Viewer + class FirefoxViewer < BaseViewer + include Exec + + CHILD_PIDS = [] + + at_exit { Process.wait if CHILD_PIDS.any? } + + trap("INT") do + CHILD_PIDS.each { |pid| Process.kill("INT", pid) } + sleep(0.5) + end + + class ProfileViewerError < StandardError; end + + VALID_COMMANDS = [ + ["which", "profile-viewer"], + ["gem", "install", "profile-viewer"], + ["profile-viewer", /.*\.json/], + ] + private_constant(:VALID_COMMANDS) + + class << self + def view(profile, params = {}) + new(profile).view(**params) + end + end + + def valid_commands + VALID_COMMANDS + end + + def initialize(profile) + super() + @profile = profile + end + + def view(_params = {}) + profile_viewer(@profile.file.to_s) + end + + private + + def setup_profile_viewer + exec("which", "profile-viewer", silent: true) do + gem_install("profile_viewer") + end + @profile_viewer_initialized = true + end + + def profile_viewer_setup + @profile_viewer_initialized || false + end + + def gem_install(gem) + exec("gem", "install", gem) do + raise ProfileViewerError, "Failed to run gem install #{gem}." + end + end + + def profile_viewer(path) + setup_profile_viewer unless profile_viewer_setup + + CHILD_PIDS << fork do + Bundler.with_unbundled_env do + exec("profile-viewer", path) do + raise ProfileViewerError, "Failed to run profile-viewer." + end + end + end + end + end + end +end diff --git a/lib/app_profiler/yarn/command.rb b/lib/app_profiler/yarn/command.rb index 2002758b..f0c989b4 100644 --- a/lib/app_profiler/yarn/command.rb +++ b/lib/app_profiler/yarn/command.rb @@ -1,8 +1,12 @@ # frozen_string_literal: true +require "app_profiler/exec" + module AppProfiler module Yarn module Command + include Exec + class YarnError < StandardError; end VALID_COMMANDS = [ @@ -17,6 +21,10 @@ class YarnError < StandardError; end private_constant(:VALID_COMMANDS) + def valid_commands + VALID_COMMANDS + end + def yarn(command, *options) setup_yarn unless yarn_setup @@ -41,22 +49,6 @@ def yarn_setup=(state) private - def ensure_command_valid(command) - unless valid_command?(command) - raise YarnError, "Illegal command: #{command.join(" ")}." - end - end - - def valid_command?(command) - VALID_COMMANDS.any? do |valid_command| - next unless valid_command.size == command.size - - valid_command.zip(command).all? do |valid_part, part| - part.match?(valid_part) - end - end - end - def ensure_yarn_installed exec("which", "yarn", silent: true) do raise( @@ -73,16 +65,6 @@ def ensure_yarn_installed def package_json_exists? AppProfiler.root.join("package.json").exist? end - - def exec(*command, silent: false) - ensure_command_valid(command) - - if silent - system(*command, out: File::NULL).tap { |return_code| yield unless return_code } - else - system(*command).tap { |return_code| yield unless return_code } - end - end end end end diff --git a/lib/app_profiler/yarn/with_firefox_profiler.rb b/lib/app_profiler/yarn/with_firefox_profiler.rb index e1e6f6fe..be92463c 100644 --- a/lib/app_profiler/yarn/with_firefox_profiler.rb +++ b/lib/app_profiler/yarn/with_firefox_profiler.rb @@ -6,7 +6,11 @@ module WithFirefoxProfiler include Command PACKAGE = "https://github.com/tenderlove/profiler#v0.0.2" - private_constant(:PACKAGE) + VALID_COMMANDS = [ + *VALID_COMMANDS, + ["git", "clone", "https://github.com/tenderlove/profiler", "firefox-profiler", "--branch=v0.0.2"], + ] + private_constant(:PACKAGE, :VALID_COMMANDS) def setup_yarn super @@ -15,6 +19,10 @@ def setup_yarn fetch_firefox_profiler end + def valid_commands + VALID_COMMANDS + end + private def firefox_profiler_added? @@ -29,7 +37,7 @@ def fetch_firefox_profiler Dir.chdir(dir) do clone_args = ["git", "clone", repo, "firefox-profiler"] clone_args.push("--branch=#{branch}") unless branch.nil? || branch&.empty? - system(*clone_args) + exec(*clone_args) package_contents = File.read("firefox-profiler/package.json") package_json = JSON.parse(package_contents) package_json["name"] ||= "firefox-profiler" diff --git a/test/app_profiler/viewer/remote/middleware/firefox_middleware_test.rb b/test/app_profiler/viewer/remote/middleware/firefox_middleware_test.rb index e57ed48a..94a123f8 100644 --- a/test/app_profiler/viewer/remote/middleware/firefox_middleware_test.rb +++ b/test/app_profiler/viewer/remote/middleware/firefox_middleware_test.rb @@ -65,19 +65,19 @@ class MiddlewareTest < TestCase end test "#call viewer sets up yarn" do - @app.expects(:system).with("which", "yarn", out: File::NULL).returns(true) - @app.expects(:system).with("yarn", "init", "--yes").returns(true) + @app.expects(:system).with({}, "which", "yarn", out: File::NULL).returns(true) + @app.expects(:system).with({}, "yarn", "init", "--yes").returns(true) url = "https://github.com/tenderlove/profiler" branch = "v0.0.2" - @app.expects(:system).with("git", "clone", url, "firefox-profiler", "--branch=#{branch}").returns(true) + @app.expects(:system).with({}, "git", "clone", url, "firefox-profiler", "--branch=#{branch}").returns(true) File.expects(:read).returns("{}") File.expects(:write).returns(true) dir = "./tmp" - @app.expects(:system).with("yarn", "--cwd", "#{dir}/firefox-profiler").returns(true) + @app.expects(:system).with({}, "yarn", "--cwd", "#{dir}/firefox-profiler").returns(true) File.expects(:read).with("#{dir}/firefox-profiler/webpack.config.js").returns("") File.expects(:write).with("#{dir}/firefox-profiler/webpack.config.js", "").returns(true) @@ -85,12 +85,12 @@ class MiddlewareTest < TestCase File.expects(:read).with("#{dir}/firefox-profiler/src/app-logic/l10n.js").returns("") File.expects(:write).with("#{dir}/firefox-profiler/src/app-logic/l10n.js", "").returns(true) - @app.expects(:system).with("yarn", "--cwd", "#{dir}/firefox-profiler", "build-prod").returns(true) + @app.expects(:system).with({}, "yarn", "--cwd", "#{dir}/firefox-profiler", "build-prod").returns(true) File.expects(:read).with("#{dir}/firefox-profiler/dist/index.html").returns("") File.expects(:write).with("#{dir}/firefox-profiler/dist/index.html", "").returns(true) - @app.expects(:system).with("yarn", "add", "--dev", "#{dir}/firefox-profiler").returns(true) + @app.expects(:system).with({}, "yarn", "add", "--dev", "#{dir}/firefox-profiler").returns(true) @app.call({ "PATH_INFO" => "/app_profiler/firefox/viewer/index.html" }) assert_predicate(@app, :yarn_setup) diff --git a/test/app_profiler/viewer/remote/middleware/speedscope_middleware_test.rb b/test/app_profiler/viewer/remote/middleware/speedscope_middleware_test.rb index 7e150e81..eff9fe84 100644 --- a/test/app_profiler/viewer/remote/middleware/speedscope_middleware_test.rb +++ b/test/app_profiler/viewer/remote/middleware/speedscope_middleware_test.rb @@ -65,10 +65,15 @@ class MiddlewareTest < TestCase end test "#call viewer sets up yarn" do - @app.expects(:system).with("which", "yarn", out: File::NULL).returns(true) - @app.expects(:system).with("yarn", "init", "--yes").returns(true) + @app.expects(:system).with({}, "which", "yarn", out: File::NULL).returns(true) + @app.expects(:system).with({}, "yarn", "init", "--yes").returns(true) @app.expects(:system).with( - "yarn", "add", "speedscope", "--dev", "--ignore-workspace-root-check" + {}, + "yarn", + "add", + "speedscope", + "--dev", + "--ignore-workspace-root-check", ).returns(true) @app.call({ "PATH_INFO" => "/app_profiler/speedscope/viewer/index.html" }) diff --git a/test/app_profiler/viewer/speedscope_viewer_test.rb b/test/app_profiler/viewer/speedscope_viewer_test.rb index fa2fcabf..77a87aea 100644 --- a/test/app_profiler/viewer/speedscope_viewer_test.rb +++ b/test/app_profiler/viewer/speedscope_viewer_test.rb @@ -16,12 +16,17 @@ class SpeedscopeViewerTest < TestCase profile = BaseProfile.from_stackprof(stackprof_profile) viewer = SpeedscopeViewer.new(profile) - viewer.expects(:system).with("which", "yarn", out: File::NULL).returns(true) - viewer.expects(:system).with("yarn", "init", "--yes").returns(true) + viewer.expects(:system).with({}, "which", "yarn", out: File::NULL).returns(true) + viewer.expects(:system).with({}, "yarn", "init", "--yes").returns(true) viewer.expects(:system).with( - "yarn", "add", "speedscope", "--dev", "--ignore-workspace-root-check" + {}, + "yarn", + "add", + "speedscope", + "--dev", + "--ignore-workspace-root-check", ).returns(true) - viewer.expects(:system).with("yarn", "run", "speedscope", profile.file.to_s).returns(true) + viewer.expects(:system).with({}, "yarn", "run", "speedscope", profile.file.to_s).returns(true) viewer.view @@ -35,11 +40,16 @@ class SpeedscopeViewerTest < TestCase AppProfiler.root.join("package.json").write("{}") viewer = SpeedscopeViewer.new(profile) - viewer.expects(:system).with("which", "yarn", out: File::NULL).returns(true) + viewer.expects(:system).with({}, "which", "yarn", out: File::NULL).returns(true) viewer.expects(:system).with( - "yarn", "add", "speedscope", "--dev", "--ignore-workspace-root-check" + {}, + "yarn", + "add", + "speedscope", + "--dev", + "--ignore-workspace-root-check", ).returns(true) - viewer.expects(:system).with("yarn", "run", "speedscope", profile.file.to_s).returns(true) + viewer.expects(:system).with({}, "yarn", "run", "speedscope", profile.file.to_s).returns(true) viewer.view @@ -55,9 +65,9 @@ class SpeedscopeViewerTest < TestCase AppProfiler.root.join("node_modules/speedscope").mkpath viewer = SpeedscopeViewer.new(profile) - viewer.expects(:system).with("which", "yarn", out: File::NULL).returns(true) - viewer.expects(:system).with("yarn", "init", "--yes").returns(true) - viewer.expects(:system).with("yarn", "run", "speedscope", profile.file.to_s).returns(true) + viewer.expects(:system).with({}, "which", "yarn", out: File::NULL).returns(true) + viewer.expects(:system).with({}, "yarn", "init", "--yes").returns(true) + viewer.expects(:system).with({}, "yarn", "run", "speedscope", profile.file.to_s).returns(true) viewer.view @@ -71,7 +81,7 @@ class SpeedscopeViewerTest < TestCase viewer = SpeedscopeViewer.new(profile) viewer.yarn_setup = true - viewer.expects(:system).with("yarn", "run", "speedscope", profile.file.to_s).returns(true) + viewer.expects(:system).with({}, "yarn", "run", "speedscope", profile.file.to_s).returns(true) viewer.view end diff --git a/test/app_profiler/yarn/command_test.rb b/test/app_profiler/yarn/command_test.rb index 11792536..adcc9f69 100644 --- a/test/app_profiler/yarn/command_test.rb +++ b/test/app_profiler/yarn/command_test.rb @@ -17,28 +17,33 @@ class CommandTest < TestCase test "#yarn allows add speedscope" do expects(:system).with( - "yarn", "add", "speedscope", "--dev", "--ignore-workspace-root-check" + {}, + "yarn", + "add", + "speedscope", + "--dev", + "--ignore-workspace-root-check", ).returns(true) yarn("add", "speedscope", "--dev", "--ignore-workspace-root-check") end test "#yarn allows init" do - expects(:system).with("yarn", "init", "--yes").returns(true) + expects(:system).with({}, "yarn", "init", "--yes").returns(true) yarn("init", "--yes") end test "#yarn allows run" do - expects(:system).with("yarn", "run", "speedscope", "\"profile.json\"").returns(true) + expects(:system).with({}, "yarn", "run", "speedscope", "\"profile.json\"").returns(true) yarn("run", "speedscope", "\"profile.json\"") end test "#yarn disallows run" do - expects(:system).with("yarn", "hack").never + expects(:system).with({}, "yarn", "hack").never - error = assert_raises(Command::YarnError) do + error = assert_raises(ArgumentError) do yarn("hack") end