search

honeybadger-io / honeybadger-ruby

Ruby gem for reporting errors to honeybadger.io
https://docs.honeybadger.io/lib/ruby/
MIT License
250 stars 146 forks source link

CI/test suite optimization #387

Closed joshuap closed 3 years ago

joshuap commented 3 years ago

The current test suite completes in ~ 2m 29s in CI. I would like to get it below ~ 1 minute.

We currently have three test suites that run with bundle exec rake: unit, integration, and features. In CI, we run the tests against a matrix of Ruby and different Gemfiles. The bulk of the test time is in the features suite, because it executes the bundle exec honeybadger CLI multiple times for end-to-end tests (it uses RSpec w/ aruba).

It's nice to test the CLI end-to-end, however I'd be open to a different approach. The CLI uses Thor, and there may be a better way to test Thor CLIs. There may be some inspiration in Thor's own test suite.

Sample Rails run:

λ  honeybadger-ruby master ✓ bundle exec appraisal rails6.0 rake
>> BUNDLE_GEMFILE=/Users/josh/Code/honeybadger-io/honeybadger-ruby/gemfiles/rails6.0.gemfile bundle exec rake
/Users/josh/.asdf/installs/ruby/3.0.0/bin/ruby -I/Users/josh/.asdf/installs/ruby/3.0.0/lib/ruby/gems/3.0.0/gems/rspec-core-3.10.1/lib:/Users/josh/.asdf/installs/ruby/3.0.0/lib/ruby/gems/3.0.0/gems/rspec-support-3.10.1/lib /Users/josh/.asdf/installs/ruby/3.0.0/lib/ruby/gems/3.0.0/gems/rspec-core-3.10.1/exe/rspec --pattern spec/unit/\*\*/\*_spec.rb --require spec_helper
Skipping AllocationStats.
Run options:
  include {:focus=>true}
  exclude {:framework=>#<Proc: ./spec/spec_helper.rb:82>}

All examples were filtered out; ignoring {:focus=>true}

Randomized with seed 34581
...............................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................................

Finished in 2.92 seconds (files took 2.08 seconds to load)
623 examples, 0 failures

Randomized with seed 34581

/Users/josh/.asdf/installs/ruby/3.0.0/bin/ruby -I/Users/josh/.asdf/installs/ruby/3.0.0/lib/ruby/gems/3.0.0/gems/rspec-core-3.10.1/lib:/Users/josh/.asdf/installs/ruby/3.0.0/lib/ruby/gems/3.0.0/gems/rspec-support-3.10.1/lib /Users/josh/.asdf/installs/ruby/3.0.0/lib/ruby/gems/3.0.0/gems/rspec-core-3.10.1/exe/rspec --pattern spec/integration/\*\*/\*_spec.rb --require spec_helper
Skipping AllocationStats.
/Users/josh/Code/honeybadger-io/honeybadger-ruby/spec/fixtures/rails/config/application.rb:5: warning: already initialized constant SKIP_ACTIVE_RECORD
/Users/josh/Code/honeybadger-io/honeybadger-ruby/spec/integration/rails_helper.rb:8: warning: previous definition of SKIP_ACTIVE_RECORD was here
Skipping Resque integration specs.
Skipping Sinatra integration specs.
Run options:
  include {:focus=>true}
  exclude {:framework=>#<Proc: ./spec/spec_helper.rb:82>}

All examples were filtered out; ignoring {:focus=>true}

Randomized with seed 18647
.........

Finished in 0.8003 seconds (files took 3.81 seconds to load)
9 examples, 0 failures

Randomized with seed 18647

/Users/josh/.asdf/installs/ruby/3.0.0/bin/ruby -I/Users/josh/.asdf/installs/ruby/3.0.0/lib/ruby/gems/3.0.0/gems/rspec-core-3.10.1/lib:/Users/josh/.asdf/installs/ruby/3.0.0/lib/ruby/gems/3.0.0/gems/rspec-support-3.10.1/lib /Users/josh/.asdf/installs/ruby/3.0.0/lib/ruby/gems/3.0.0/gems/rspec-core-3.10.1/exe/rspec --pattern spec/features/\*\*/\*_spec.rb --require spec_helper
Skipping AllocationStats.
** [Honeybadger] Initializing Honeybadger Error Tracker for Ruby. Ship it! version=4.7.3 framework=ruby level=1 pid=35368
Run options:
  include {:focus=>true}
  exclude {:framework=>#<Proc: ./spec/spec_helper.rb:82>}

All examples were filtered out; ignoring {:focus=>true}

Randomized with seed 50176
..................................................

Finished in 1 minute 32.52 seconds (files took 1.36 seconds to load)
50 examples, 0 failures

Randomized with seed 50176
KonnorRogers commented 3 years ago

Another performance win could also be looking at Gems in the Gemfile.

Right now the longest running install time I see is just over a minute.

Perhaps we could look at programmatically pulling in Rails dependencies. For example, I did a quick grep on the repo and didnt see a single instance of Sprockets or asset[s]. I think itd be safe to assume we dont need sprocket-rails and by extension sprockets which also pulls in sassc which needs to be compiled which is never a good time.

I dont have enough insight into Honeybadger, but we could look over the dependencies here:

https://rubygems.org/gems/rails/versions/6.0.0

and see what we really need and what could be cut and pull in accordingly.

KonnorRogers commented 3 years ago

As for the CLI itself, I've poked and prodded around the spec files a bit. It seems the most efficient way is calling: Klass.start(%w(command subcommand)) where Klass inherits from Thor.

https://github.com/erikhuda/thor/blob/master/spec/runner_spec.rb https://github.com/erikhuda/thor/blob/master/spec/subcommand_spec.rb

They also have some suite setup available here to provide nice helpers:

https://github.com/erikhuda/thor/blob/master/spec/helper.rb

headius commented 3 years ago

The JRuby failures may be fixed by using a newer JRuby.

It appears the builds are using JRuby 9.2.9.0 which is pretty old at this point, having been released well over a year ago. The Rails 5.2 build fails with one internal exception that should be fixed as of 9.2.14.0. The same error occurs in the Rails 6 build, along with some others that I did not try to diagnose. At a minimum you should try updating to 9.2.14.0.

It seems that the default rvm in GHA has very old versions, since you are just using the default jruby and it is 9.2.9.0. I believe you will want to rvm get head to update it before installing. It is unfortunate that the GHA rvm plugin does not use latest.

If there are still failures on JRuby ping me.

headius commented 3 years ago

You could also force rvm to install 9.2.14.0, I think, using jruby-9.2.14.0. However you'll have to manually update that as JRuby versions are released.

KonnorRogers commented 3 years ago

@headius this was related to the old setup-ruby. Currently the new GH-action supports all the way up to 9.2.14.0 and HEAD https://github.com/ruby/setup-ruby#setup-ruby

joshuap commented 3 years ago

I think it would be nice to rewrite the CLI (aruba) tests as Thor tests so that they don't spawn ruby a bunch of times. I'm curious how much time that would take—I don't want to spend an unlimited amount of time on it.

One other smaller optimization that comes to mind is that we could potentially parallelize the three rake tasks, since there is some startup cost to running three test suites instead of one.

Beyond that, it does feel like we've reached diminishing returns.

joshuap commented 3 years ago

Update: let's spend a few hours on this approach to Thor CLI testing to see if it's a viable direction or not. 👍

joshuap commented 3 years ago

The individual test runs themselves are each below 1 minute. The "Setup JRuby" step is still taking ~1m 30s for each JRuby build, which adds to the overall CI time. By comparison, "Setup Ruby" on the Ruby 3.0 build adds only ~25s.

We may be able to further optimize GitHub Actions in the future, and we now have a way to test Thor commands without spawning a new process for each test. I think that's good for now, closing this!