Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Local firefox viewer #144

Merged
merged 1 commit into from
Nov 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion lib/app_profiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
Expand Down
37 changes: 37 additions & 0 deletions lib/app_profiler/exec.rb
Original file line number Diff line number Diff line change
@@ -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
79 changes: 79 additions & 0 deletions lib/app_profiler/viewer/firefox_viewer.rb
Original file line number Diff line number Diff line change
@@ -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
34 changes: 8 additions & 26 deletions lib/app_profiler/yarn/command.rb
Original file line number Diff line number Diff line change
@@ -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 = [
Expand All @@ -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

Expand All @@ -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(
Expand All @@ -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
12 changes: 10 additions & 2 deletions lib/app_profiler/yarn/with_firefox_profiler.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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"],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for a future PR, i wonder if we could just use the gem and extract the sources from that. This branch is out of date and doesn't have the latest changes already.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I'll try to account for this in another PR.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We make changes to the source so I don't think extracting from the gem is the best idea, unless we plan to copy it somewhere.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes are basically just so we can compile it as a package IIRC. We might be able to get away with just extracting the sources from the gem as-is?

]
private_constant(:PACKAGE, :VALID_COMMANDS)

def setup_yarn
super
Expand All @@ -15,6 +19,10 @@ def setup_yarn
fetch_firefox_profiler
end

def valid_commands
VALID_COMMANDS
end

private

def firefox_profiler_added?
Expand All @@ -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"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,32 +65,32 @@ 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)

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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" })
Expand Down
32 changes: 21 additions & 11 deletions test/app_profiler/viewer/speedscope_viewer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand All @@ -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

Expand All @@ -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
Expand Down
15 changes: 10 additions & 5 deletions test/app_profiler/yarn/command_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Loading