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.

Sign up for GitHub

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

Jump to bottom

Bye byebug, hi debug! #17010

Merged
merged 8 commits into from
Apr 18, 2024
Merged

Bye byebug, hi debug! #17010

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
11c1960
Remove `byebug` since it doesn't work in tests anymore
issyl0 Apr 2, 2024
58fbaec
Add the `debug` gem for test debugging
issyl0 Apr 2, 2024
1daeb5b
Set up the `debug` gem for test debugging
issyl0 Apr 2, 2024
f2a64f3
Fix RuboCop and exclude `psych` from RBI generation
issyl0 Apr 2, 2024
621e51a
Don't `require "debug"` in `brew tests` - it's in our RSpec config
issyl0 Apr 4, 2024
073e739
Use the `debug` gem from portable Ruby
issyl0 Apr 7, 2024
9cf5334
Load `debug.so` extensions along with the debug gem itself
issyl0 Apr 10, 2024
4c64193
Set `HOMEBREW_DEBUG` when running `brew tests --debug`
issyl0 Apr 17, 2024
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
2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
/Library/Taps
/Library/PinnedTaps
/Library/Homebrew/.byebug_history
/Library/Homebrew/test/.rdbg_history

# Ignore Bundler files
**/.bundle/bin
Expand Down Expand Up @@ -105,6 +106,7 @@
**/vendor/bundle/ruby/*/gems/racc-*/
**/vendor/bundle/ruby/*/gems/rainbow-*/
**/vendor/bundle/ruby/*/gems/rbi-*/
**/vendor/bundle/ruby/*/gems/rdoc-*/
**/vendor/bundle/ruby/*/gems/regexp_parser-*/
**/vendor/bundle/ruby/*/gems/rexml-*/
**/vendor/bundle/ruby/*/gems/rspec-*/
Expand Down
1 change: 0 additions & 1 deletion Library/Homebrew/Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@ group :style, optional: true do
gem "rubocop-sorbet", require: false
end
group :tests, optional: true do
gem "byebug", require: false
gem "parallel_tests", require: false
gem "rspec", require: false
gem "rspec-github", require: false
Expand Down
2 changes: 0 additions & 2 deletions Library/Homebrew/Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ GEM
bindata (2.5.0)
bootsnap (1.18.3)
msgpack (~> 1.2)
byebug (11.1.3)
coderay (1.1.3)
commander (4.6.0)
highline (~> 2.0.0)
Expand Down Expand Up @@ -167,7 +166,6 @@ PLATFORMS
DEPENDENCIES
addressable
bootsnap
byebug
json_schemer
kramdown
method_source
Expand Down
8 changes: 4 additions & 4 deletions Library/Homebrew/dev-cmd/tests.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,8 @@ class Tests < AbstractCommand
switch "--online",
description: "Include tests that use the GitHub API and tests that use any of the taps for " \
"official external commands."
switch "--byebug",
description: "Enable debugging using byebug."
switch "--debug",
issyl0 marked this conversation as resolved.
Show resolved Hide resolved
description: "Enable debugging using ruby/debug, or surface the standard `odebug` output."
switch "--changed",
description: "Only runs tests on files that were changed from the master branch."
switch "--fail-fast",
Expand All @@ -45,8 +45,6 @@ def run
# Given we might be testing various commands, we probably want everything (except sorbet-static)
Homebrew.install_bundler_gems!(groups: Homebrew.valid_gem_groups - ["sorbet"])

require "byebug" if args.byebug?

HOMEBREW_LIBRARY_PATH.cd do
setup_environment!

Expand Down Expand Up @@ -137,6 +135,8 @@ def run

puts "Randomized with seed #{seed}"

ENV["HOMEBREW_DEBUG"] = "1" if args.debug? # Used in spec_helper.rb to require the "debug" gem.

# Submit test flakiness information using BuildPulse
# BUILDPULSE used in spec_helper.rb
if use_buildpulse?
Expand Down
3 changes: 0 additions & 3 deletions Library/Homebrew/sorbet/rbi/dsl/homebrew/dev_cmd/tests.rbi
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Library/Homebrew/sorbet/tapioca/config.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ gem:
- json
- msgpack
# These aren't needed:
- byebug
- coderay
- commander
- diff-lcs
Expand Down
8 changes: 8 additions & 0 deletions Library/Homebrew/standalone/init.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,14 @@
ENV["HOMEBREW_SKIP_INITIAL_GEM_INSTALL"] = "1"
end

if Pathname.new(RbConfig.ruby).to_s.include?("/vendor/portable-ruby/")
ruby_version = RbConfig::CONFIG["ruby_version"]
ruby_path = "#{RbConfig::CONFIG["rubylibprefix"]}/gems/#{ruby_version}"

$LOAD_PATH.unshift "#{ruby_path}/extensions/#{Gem::Platform.local}/#{ruby_version}-static/debug-1.6.3"
Copy link
Member

Choose a reason for hiding this comment

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

Might be nice to glob or use a Gem method here so the version isn't hardcoded.

$LOAD_PATH.unshift "#{ruby_path}/gems/debug-1.6.3/lib"
end

unless $LOAD_PATH.include?(HOMEBREW_LIBRARY_PATH.to_s)
# Insert the path after any existing Homebrew paths (e.g. those inserted by tests and parent processes)
last_homebrew_path_idx = $LOAD_PATH.rindex do |path|
Expand Down
9 changes: 5 additions & 4 deletions Library/Homebrew/test/spec_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@
require "rspec/sorbet"
require "rubocop/rspec/support"
require "find"
require "byebug"
require "timeout"

$LOAD_PATH.unshift(File.expand_path("#{ENV.fetch("HOMEBREW_LIBRARY")}/Homebrew/test/support/lib"))
Expand All @@ -38,6 +37,8 @@

require_relative "../global"

require "debug" if ENV["HOMEBREW_DEBUG"]
Copy link
Member

Choose a reason for hiding this comment

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

i wonder if we should just do this for any command run with --debug?

Copy link
Member

@Bo98 Bo98 Apr 22, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Unlike brew tests which is a developer-only command, we ask end users to run --debug and we do not want this behaviour for them. Or if we do then it should catch a LoadError and fail silently.


require "test/support/quiet_progress_formatter"
require "test/support/helper/cask"
require "test/support/helper/files"
Expand Down Expand Up @@ -230,14 +231,14 @@
@__stdin = $stdin.clone

begin
if !example.metadata.keys.intersect?([:focus, :byebug]) && !ENV.key?("HOMEBREW_VERBOSE_TESTS")
if example.metadata.keys.exclude?(:focus) && !ENV.key?("HOMEBREW_VERBOSE_TESTS")
$stdout.reopen(File::NULL)
$stderr.reopen(File::NULL)
$stdin.reopen(File::NULL)
else
# don't retry when focusing/debugging
# don't retry when focusing
config.default_retry_count = 0
end
$stdin.reopen(File::NULL)

begin
timeout = example.metadata.fetch(:timeout, 60)
Expand Down
2 changes: 0 additions & 2 deletions Library/Homebrew/vendor/bundle/bundler/setup.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ def self.extension_api_version
$:.unshift File.expand_path("#{__dir__}/../#{RUBY_ENGINE}/#{Gem.ruby_api_version}/gems/msgpack-1.7.2/lib")
$:.unshift File.expand_path("#{__dir__}/../#{RUBY_ENGINE}/#{Gem.ruby_api_version}/extensions/arm64-darwin-20/#{Gem.extension_api_version}/bootsnap-1.18.3")
$:.unshift File.expand_path("#{__dir__}/../#{RUBY_ENGINE}/#{Gem.ruby_api_version}/gems/bootsnap-1.18.3/lib")
$:.unshift File.expand_path("#{__dir__}/../#{RUBY_ENGINE}/#{Gem.ruby_api_version}/extensions/arm64-darwin-20/#{Gem.extension_api_version}/byebug-11.1.3")
$:.unshift File.expand_path("#{__dir__}/../#{RUBY_ENGINE}/#{Gem.ruby_api_version}/gems/byebug-11.1.3/lib")
$:.unshift File.expand_path("#{__dir__}/../#{RUBY_ENGINE}/#{Gem.ruby_api_version}/gems/coderay-1.1.3/lib")
$:.unshift File.expand_path("#{__dir__}/../#{RUBY_ENGINE}/#{Gem.ruby_api_version}/gems/highline-2.0.3/lib")
$:.unshift File.expand_path("#{__dir__}/../#{RUBY_ENGINE}/#{Gem.ruby_api_version}/gems/commander-4.6.0/lib")
Expand Down
1 change: 0 additions & 1 deletion completions/bash/brew
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2208,7 +2208,6 @@ _brew_tests() {
case "${cur}" in
-*)
__brewcomp "
--byebug
--changed
--coverage
--debug
Expand Down
3 changes: 1 addition & 2 deletions completions/fish/brew.fish
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1465,10 +1465,9 @@ __fish_brew_complete_arg 'test' -a '(__fish_brew_suggest_formulae_installed)'


__fish_brew_complete_cmd 'tests' 'Run Homebrew\'s unit and integration tests'
__fish_brew_complete_arg 'tests' -l byebug -d 'Enable debugging using byebug'
__fish_brew_complete_arg 'tests' -l changed -d 'Only runs tests on files that were changed from the master branch'
__fish_brew_complete_arg 'tests' -l coverage -d 'Generate code coverage reports'
__fish_brew_complete_arg 'tests' -l debug -d 'Display any debugging information'
__fish_brew_complete_arg 'tests' -l debug -d 'Enable debugging using ruby/debug, or surface the standard `odebug` output'
__fish_brew_complete_arg 'tests' -l fail-fast -d 'Exit early on the first failing test'
__fish_brew_complete_arg 'tests' -l generic -d 'Run only OS-agnostic tests'
__fish_brew_complete_arg 'tests' -l help -d 'Show this message'
Expand Down
3 changes: 1 addition & 2 deletions completions/zsh/_brew
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1815,10 +1815,9 @@ _brew_test() {
# brew tests
_brew_tests() {
_arguments \
'--byebug[Enable debugging using byebug]' \
'(--only)--changed[Only runs tests on files that were changed from the master branch]' \
'--coverage[Generate code coverage reports]' \
'--debug[Display any debugging information]' \
'--debug[Enable debugging using ruby/debug, or surface the standard `odebug` output]' \
'--fail-fast[Exit early on the first failing test]' \
'--generic[Run only OS-agnostic tests]' \
'--help[Show this message]' \
Expand Down
4 changes: 2 additions & 2 deletions docs/Manpage.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2650,9 +2650,9 @@ Run Homebrew's unit and integration tests.
: Include tests that use the GitHub API and tests that use any of the taps for
official external commands.

`--byebug`
`--debug`

: Enable debugging using byebug.
: Enable debugging using ruby/debug, or surface the standard `odebug` output.

`--changed`

Expand Down
4 changes: 2 additions & 2 deletions manpages/brew.1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1687,8 +1687,8 @@ Run only OS\-agnostic tests\.
\fB\-\-online\fP
Include tests that use the GitHub API and tests that use any of the taps for official external commands\.
.TP
\fB\-\-byebug\fP
Enable debugging using byebug\.
\fB\-\-debug\fP
Enable debugging using ruby/debug, or surface the standard \fBodebug\fP output\.
.TP
\fB\-\-changed\fP
Only runs tests on files that were changed from the master branch\.
Expand Down