Merge pull request #32 from github/bump-0-9-0

Bump to version 0.9.0
2024-04-18 13:53:10 -04:00 · 2024-04-18 13:53:10 -04:00 · 5dca1f1136
--- a/.github/workflows/ruby.yml
+++ b/.github/workflows/ruby.yml
@ -6,17 +6,18 @@ on:
  pull_request:
    branches:
    - main
-permissions:
-  contents: read
 jobs:
  test:
    runs-on: ubuntu-latest
    strategy:
+      fail-fast: false
      matrix:
        ruby-version:
-        - '2.7'
-        - '3.0'
-        - '3.1'
+        - "3.0"
+        - "3.1"
+        - "3.2"
+        - "3.3"
+        - head
        gemfile-path:
        - Gemfile
        - gemfiles/faraday_0.gemfile
@ -25,11 +26,11 @@ jobs:
    env:
      BUNDLE_GEMFILE: ${{ github.workspace }}/${{ matrix.gemfile-path }}
    steps:
-    - uses: actions/checkout@v3
+    - uses: actions/checkout@v4
    - name: Set up Ruby
      uses: ruby/setup-ruby@v1
      with:
        ruby-version: ${{ matrix.ruby-version }}
        bundler-cache: true
    - name: Run tests
-      run: bundle exec rake
+      run: bin/test
--- a/.gitignore
+++ b/.gitignore
@ -1,13 +1,11 @@
-*.gem
 /.bundle/
 /.yardoc
-/Gemfile.lock
-/gemfiles/*.lock
 /_yardoc/
 /coverage/
 /doc/
 /pkg/
 /spec/reports/
 /tmp/
-/vendor/gems
-.ruby-version
+
+/Gemfile.lock
+/gemfiles/*.lock
--- a/.rubocop.yml
+++ b/.rubocop.yml
@ -5,44 +5,27 @@ require:

 AllCops:
  NewCops: enable
-  TargetRubyVersion: 2.7
+  TargetRubyVersion: 3.0

 Layout/MultilineMethodCallIndentation:
-  Enabled: true
  EnforcedStyle: indented_relative_to_receiver

 Metrics:
  Enabled: false

-Minitest/AssertEmptyLiteral:
-  Enabled: false
 Minitest/AssertEqual:
  Enabled: false
-Minitest/AssertPredicate:
-  Enabled: false
 Minitest/MultipleAssertions:
  Enabled: false
-Minitest/RefutePredicate:
-  Enabled: false

 Naming/RescuedExceptionsVariableName:
-  Enabled: true
  PreferredName: error

-Style/ClassAndModuleChildren:
-  Enabled: true
-  Exclude:
-  - test/**/*
 Style/Documentation:
  Enabled: false
-Style/IfUnlessModifier:
-  Enabled: false
-Style/StringLiterals:
-  Enabled: true
-  EnforcedStyle: double_quotes
-Style/SymbolArray:
-  Enabled: false
 Style/InfiniteLoop:
  Enabled: false
 Style/Next:
  Enabled: false
+Style/StringLiterals:
+  EnforcedStyle: double_quotes
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@ -0,0 +1,131 @@
+# Changelog
+
+All notable changes to this project will be documented in this file.
+
+The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
+and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
+
+## [Unreleased]
+
+## [0.9.0] - 2024-04-18
+
+### Added
+
+- Support for Ruby versions 3.2 and 3.3
+
+### Changed
+
+- Move RuboCop configuration closer to defaults
+- Treat throttler max wait time as a hard cap
+
+### Removed
+
+- Support for EOL Ruby version 2.7
+
+## [0.8.3] - 2024-04-18
+
+### Fixed
+
+- Prevent infinite loops when throttling an enumerator
+
+## [0.8.2] - 2022-10-20
+
+### Added
+
+- Support for Ruby versions 3.0 and 3.1
+- Support for Faraday versions 1.x and 2.x
+
+### Changed
+
+- Moved build to GitHub Actions
+- Update RuboCop to target Ruby 2.7
+
+### Removed
+
+- Support for EOL Ruby versions 2.5 and 2.6
+
+## [0.8.1] - 2020-06-09
+
+### Added
+
+- Ability to add a single decorator
+
+## [0.8.0] - 2020-06-09
+
+### Added
+
+- Support for Ruby versions 2.6 and 2.7
+- Support for Freno's "low priority" checks
+
+### Changed
+
+- Update RuboCop to target Ruby 2.5
+- Changed authorship to "GitHub"
+
+### Removed
+
+- Support for EOL Ruby versions 2.3 and 2.4
+
+## [0.7.0] - 2019-01-23
+
+### Added
+
+- Support for Ruby version 2.7
+
+### Changed
+
+- Prevent decorator reuse
+
+### Removed
+
+- Support for EOL Ruby versions 2.1 and 2.2
+
+## [0.6.0] - 2017-10-09
+
+### Added
+
+- Throttlers!
+
+## [0.5.0] - 2017-10-09
+
+### Added
+
+- Custom error classes
+
+### Changed
+
+- Simplify gem release and its documentation
+
+## [0.4.0] - 2017-08-29
+
+### Added
+
+- RuboCop configuration and build step
+- Wrapping raised errors in Freno::Error
+
+## [0.3.0] - 2017-07-07
+
+### Changed
+
+- Require a Ruby version 2.0 or greater
+- Relax Faraday's version requirement
+
+## [0.2.0] - 2017-07-07
+
+### Added
+
+- Initial import
+- Change ownership and contact information to GitHub
+
+[unreleased]: https://github.com/github/freno-client/compare/v0.9.0...HEAD
+[0.9.0]: https://github.com/github/freno-client/compare/v0.8.3...v0.9.0
+[0.8.3]: https://github.com/github/freno-client/compare/v0.8.2...v0.8.3
+[0.8.2]: https://github.com/github/freno-client/compare/v0.8.1...v0.8.2
+[0.8.1]: https://github.com/github/freno-client/compare/v0.8.0...v0.8.1
+[0.8.0]: https://github.com/github/freno-client/compare/v0.7.0...v0.8.0
+[0.7.0]: https://github.com/github/freno-client/compare/v0.6.0...v0.7.0
+[0.6.0]: https://github.com/github/freno-client/compare/v0.5.0...v0.6.0
+[0.5.0]: https://github.com/github/freno-client/compare/v0.4.0...v0.5.0
+[0.4.0]: https://github.com/github/freno-client/compare/v0.3.0...v0.4.0
+[0.3.0]: https://github.com/github/freno-client/compare/v0.2.0...v0.3.0
+[0.2.0]: https://github.com/github/freno-client/commits/v0.2.0
--- a/19
+++ b/19
@ -4,15 +4,10 @@ source "https://rubygems.org"

 gemspec

-group :development do
-  gem "rake"
-end
-
-group :test do
-  gem "minitest", ">= 5"
-  gem "mocha"
-  gem "rubocop", "~> 1.37", require: false
-  gem "rubocop-minitest", require: false
-  gem "rubocop-performance", require: false
-  gem "rubocop-rake", require: false
-end
+gem "minitest", "~> 5.22"
+gem "mocha", "~> 2.2"
+gem "rake", "~> 13.2"
+gem "rubocop", "~> 1.63", require: false
+gem "rubocop-minitest", "~> 0.35.0", require: false
+gem "rubocop-performance", "~> 1.21", require: false
+gem "rubocop-rake", "~> 0.6.0", require: false
--- a/11
+++ b/11
@ -1,15 +1,10 @@
 # frozen_string_literal: true

 require "bundler/gem_tasks"
-require "rake/testtask"
+require "minitest/test_task"
 require "rubocop/rake_task"

-Rake::TestTask.new(:test) do |t|
-  t.libs << "test"
-  t.libs << "lib"
-  t.test_files = FileList["test/**/*_test.rb"]
-end
-
+Minitest::TestTask.create(:test)
 RuboCop::RakeTask.new(:rubocop)

-task default: [:test, :rubocop]
+task default: %i[test rubocop]
--- a/freno-client.gemspec
+++ b/freno-client.gemspec
@ -3,26 +3,31 @@
 require_relative "lib/freno/client/version"

 Gem::Specification.new do |spec|
-  spec.name    = "freno-client"
+  spec.name = "freno-client"
  spec.version = Freno::Client::VERSION
-  spec.author  = "GitHub"
-  spec.email   = "opensource+freno-client@github.com"
-
-  spec.summary     = "A library for interacting with Freno, the throttler service"
+  spec.summary = "A library for interacting with Freno, the throttler service"
  spec.description = <<~DESC.gsub(/\s+/, " ")
    freno-client is a Ruby library that interacts with Freno using HTTP.
    Freno is a throttling service and its source code is available at
    https://github.com/github/freno
  DESC

+  spec.author = "GitHub"
+  spec.email = "opensource+freno-client@github.com"
+  spec.license = "MIT"
  spec.homepage = "https://github.com/github/freno-client"
-  spec.license  = "MIT"

-  spec.required_ruby_version = ">= 2.7.0"
-
-  spec.files = `git ls-files -z`.split("\x0").grep_v(/^(bin|test)/)
+  spec.metadata = {
+    "allowed_push_host" => "https://rubygems.org",
+    "bug_tracker_uri" => "https://github.com/github/freno-client/issues",
+    "homepage_uri" => "https://github.com/github/freno-client",
+    "rubygems_mfa_required" => "true",
+    "source_code_uri" => "https://github.com/github/freno-client"
+  }

+  spec.required_ruby_version = ">= 3.0"
  spec.add_dependency "faraday", "< 3"

-  spec.metadata["rubygems_mfa_required"] = "true"
+  spec.files = Dir.glob(["freno-client.gemspec", "lib/**/*.rb", "LICENSE.txt"])
+  spec.extra_rdoc_files = ["README.md"]
 end
--- a/gemfiles/faraday_0.gemfile
+++ b/gemfiles/faraday_0.gemfile
@ -8,15 +8,10 @@ gem "faraday", "~> 0.0"

 # Content below copied from Gemfile

-group :development do
-  gem "rake"
-end
-
-group :test do
-  gem "minitest", ">= 5"
-  gem "mocha"
-  gem "rubocop", "~> 1.37", require: false
-  gem "rubocop-minitest", require: false
-  gem "rubocop-performance", require: false
-  gem "rubocop-rake", require: false
-end
+gem "minitest", "~> 5.22"
+gem "mocha", "~> 2.2"
+gem "rake", "~> 13.2"
+gem "rubocop", "~> 1.63", require: false
+gem "rubocop-minitest", "~> 0.35.0", require: false
+gem "rubocop-performance", "~> 1.21", require: false
+gem "rubocop-rake", "~> 0.6.0", require: false
--- a/gemfiles/faraday_1.gemfile
+++ b/gemfiles/faraday_1.gemfile
@ -8,15 +8,10 @@ gem "faraday", "~> 1.0"

 # Content below copied from Gemfile

-group :development do
-  gem "rake"
-end
-
-group :test do
-  gem "minitest", ">= 5"
-  gem "mocha"
-  gem "rubocop", "~> 1.37", require: false
-  gem "rubocop-minitest", require: false
-  gem "rubocop-performance", require: false
-  gem "rubocop-rake", require: false
-end
+gem "minitest", "~> 5.22"
+gem "mocha", "~> 2.2"
+gem "rake", "~> 13.2"
+gem "rubocop", "~> 1.63", require: false
+gem "rubocop-minitest", "~> 0.35.0", require: false
+gem "rubocop-performance", "~> 1.21", require: false
+gem "rubocop-rake", "~> 0.6.0", require: false
--- a/gemfiles/faraday_2.gemfile
+++ b/gemfiles/faraday_2.gemfile
@ -8,15 +8,10 @@ gem "faraday", "~> 2.0"

 # Content below copied from Gemfile

-group :development do
-  gem "rake"
-end
-
-group :test do
-  gem "minitest", ">= 5"
-  gem "mocha"
-  gem "rubocop", "~> 1.37", require: false
-  gem "rubocop-minitest", require: false
-  gem "rubocop-performance", require: false
-  gem "rubocop-rake", require: false
-end
+gem "minitest", "~> 5.22"
+gem "mocha", "~> 2.2"
+gem "rake", "~> 13.2"
+gem "rubocop", "~> 1.63", require: false
+gem "rubocop-minitest", "~> 0.35.0", require: false
+gem "rubocop-performance", "~> 1.21", require: false
+gem "rubocop-rake", "~> 0.6.0", require: false
--- a/lib/freno/client/preconditions.rb
+++ b/lib/freno/client/preconditions.rb
@ -16,9 +16,7 @@ module Freno

        def present(args = {})
          args.each do |arg, value|
-            unless value
-              errors << "#{arg} should be present"
-            end
+            errors << "#{arg} should be present" unless value
          end
        end

--- a/lib/freno/client/version.rb
+++ b/lib/freno/client/version.rb
@ -2,6 +2,6 @@

 module Freno
  class Client
-    VERSION = "0.8.3"
+    VERSION = Gem::Version.new("0.9.0")
  end
 end
--- a/lib/freno/throttler.rb
+++ b/lib/freno/throttler.rb
@ -178,14 +178,14 @@ module Freno
          break
        end

-        wait
-        waited += wait_seconds
-        instrument(:waited, store_names: store_names, waited: waited, max: max_wait_seconds)
-
-        if waited > max_wait_seconds
+        if waited + wait_seconds > max_wait_seconds
          instrument(:waited_too_long, store_names: store_names, waited: waited, max: max_wait_seconds)
          circuit_breaker.failure
          raise WaitedTooLong.new(waited_seconds: waited, max_wait_seconds: max_wait_seconds)
+        else
+          wait
+          waited += wait_seconds
+          instrument(:waited, store_names: store_names, waited: waited, max: max_wait_seconds)
        end
      end

--- a/test/freno/client/requests/check_read_test.rb
+++ b/test/freno/client/requests/check_read_test.rb
@ -2,7 +2,7 @@

 require "test_helper"

-class Freno::Client::Requests::CheckReadTest < Freno::Client::Test
+class FrenoClientRequestsCheckReadTest < ClientTest
  include Freno::Client::Requests

  def test_preconditions_require_an_app_to_be_present
--- a/test/freno/client/requests/check_test.rb
+++ b/test/freno/client/requests/check_test.rb
@ -2,7 +2,7 @@

 require "test_helper"

-class Freno::Client::Requests::CheckTest < Freno::Client::Test
+class FrenoClientRequestsCheckTest < ClientTest
  include Freno::Client::Requests

  def test_preconditions_require_an_app_to_be_present
--- a/test/freno/client_test.rb
+++ b/test/freno/client_test.rb
@ -2,7 +2,7 @@

 require "test_helper"

-class Freno::ClientTest < Freno::Client::Test
+class FrenoClientTest < ClientTest
  def test_that_it_has_a_version_number
    refute_nil ::Freno::Client::VERSION
  end
@ -24,7 +24,7 @@ class Freno::ClientTest < Freno::Client::Test

    assert_operator client.check, :==, :ok
    assert_operator client.check, :==, 200
-    assert client.check?
+    assert_predicate client, :check?
  end

  def test_check_fails
@ -34,7 +34,7 @@ class Freno::ClientTest < Freno::Client::Test

    assert_operator client.check, :==, :internal_server_error
    assert_operator client.check, :==, 500
-    refute client.check?
+    refute_predicate client, :check?
  end

  def test_check_read_succeeds
@ -125,7 +125,7 @@ class Freno::ClientTest < Freno::Client::Test
    memo.clear

    assert_operator client.check, :==, :ok
-    assert_equal [], memo
+    assert_empty memo
  end

  def test_decorators_can_be_applied_to_all_requests
--- a/test/freno/throttler_test.rb
+++ b/test/freno/throttler_test.rb
@ -2,7 +2,7 @@

 require "test_helper"

-class Freno::ThrottlerTest < Freno::Throttler::Test
+class FrenoThrottlerTest < ThrottlerTest
  def test_validations
    ex = assert_raises(ArgumentError) do
      Freno::Throttler.new(wait_seconds: 1, max_wait_seconds: 0.5)
@ -63,12 +63,12 @@ class Freno::ThrottlerTest < Freno::Throttler::Test
    assert block_called, "block should have been called"

    assert_equal 1, throttler.instrumenter.count("throttler.called")
-    assert_equal [], throttler.instrumenter.events_for("throttler.called")
-                       .first[:store_names]
+    assert_empty throttler.instrumenter.events_for("throttler.called")
+                   .first[:store_names]

    assert_equal 1, throttler.instrumenter.count("throttler.succeeded")
-    assert_equal [], throttler.instrumenter.events_for("throttler.succeeded")
-                       .first[:store_names]
+    assert_empty throttler.instrumenter.events_for("throttler.succeeded")
+                   .first[:store_names]

    assert_equal 0, throttler.instrumenter.count("throttler.waited")
    assert_equal 0, throttler.instrumenter.count("throttler.waited_too_long")
@ -90,7 +90,7 @@ class Freno::ThrottlerTest < Freno::Throttler::Test
      t.mapper = ->(_context) { [:mysqla] }
      t.instrumenter = MemoryInstrumenter.new
    end
-    throttler.expects(:wait).once.returns(0.1)
+    throttler.expects(:wait).once

    throttler.throttle do
      block_called = true
@ -128,11 +128,11 @@ class Freno::ThrottlerTest < Freno::Throttler::Test
      t.app = :github
      t.mapper = ->(_context) { [:mysqla] }
      t.instrumenter = MemoryInstrumenter.new
-      t.wait_seconds = 0.1
-      t.max_wait_seconds = 0.3
+      t.wait_seconds = 1
+      t.max_wait_seconds = 3
    end

-    throttler.expects(:wait).times(3).returns(0.1)
+    throttler.expects(:wait).times(3)

    assert_raises(Freno::Throttler::WaitedTooLong) do
      throttler.throttle do
@ -151,8 +151,8 @@ class Freno::ThrottlerTest < Freno::Throttler::Test

    assert_equal 1, waited_too_long_events.count
    assert_equal [:mysqla], waited_too_long_events.first[:store_names]
-    assert_in_delta 0.3, waited_too_long_events.first[:max], 0.01
-    assert_operator waited_too_long_events.first[:waited], :>=, 0.3
+    assert_equal 3, waited_too_long_events.first[:max]
+    assert_equal 3, waited_too_long_events.first[:waited]

    assert_equal 0, throttler.instrumenter.count("throttler.freno_errored")
    assert_equal 0, throttler.instrumenter.count("throttler.circuit_open")
@ -250,4 +250,103 @@ class Freno::ThrottlerTest < Freno::Throttler::Test
      end
    end
  end
+
+  def test_throttles_an_enumerator
+    array = [1, 2, 3]
+    enumerator = array.each
+    result = []
+
+    throttler = Freno::Throttler.new(client: sample_client, app: :github)
+
+    begin
+      Timeout.timeout(0.1) do
+        loop do
+          throttler.throttle do
+            result << enumerator.next
+          end
+        end
+      end
+    rescue Timeout::Error
+      flunk "Throttling an enumerator caused an infinite loop."
+    end
+
+    assert_equal array, result
+  end
+
+  # This test ensures that a throttle call will not wait if that wait would
+  # not be followed by another check, making that wait time useless. For
+  # example, consider a throttler with 1s wait time and 3s max wait time:
+  #
+  # C = check all stores
+  # - = 100ms of wait time
+  # X = raise waited too long
+  #
+  #       0s         1s         2s         3s         4s
+  #       ├──────────┼──────────┼──────────┼──────────┤
+  # v0.8: C----------C----------C----------C----------X
+  # v0.9: C----------C----------C----------CX
+  #
+  def test_does_not_wait_longer_than_needed
+    block_called = false
+    client = sample_client
+
+    throttler = Freno::Throttler.new(
+      client: client,
+      app: :github,
+      wait_seconds: 1,
+      max_wait_seconds: 3
+    )
+
+    # We expect to check four times with three
+    # one-second waits between the attempts.
+    client.stubs(:check?).times(4).returns(false)
+    throttler.expects(:wait).times(3)
+
+    assert_raises(Freno::Throttler::WaitedTooLong) do
+      throttler.throttle(:mysqla) do
+        block_called = true
+      end
+    end
+
+    refute block_called, "block should not have been called"
+  end
+
+  # This test ensures that a throttle call will not wait longer than the
+  # configured maximum wait time, even when that maximum doesn't divide by
+  # the configured wait time evenly. For example, consider a throttler with
+  # 2s wait time and 5s max wait time:
+  #
+  # C = check all stores
+  # - = 100ms of wait time
+  # X = raise waited too long
+  #                                             max_wait_seconds ↴
+  #       0s         1s         2s         3s         4s         5s         6s
+  #       ├──────────┼──────────┼──────────┼──────────┼──────────┼──────────┤
+  # v0.8: C---------------------C---------------------C---------------------CX
+  # v0.9: C---------------------C---------------------CX
+  #
+  def test_does_not_exceed_max_wait_time
+    block_called = false
+    client = sample_client
+
+    throttler = Freno::Throttler.new(
+      client: client,
+      app: :github,
+      wait_seconds: 2,
+      max_wait_seconds: 5
+    )
+
+    # We expect to check four times with three
+    # one-second waits between the attempts.
+    client.stubs(:check?).times(3).returns(false)
+    throttler.expects(:wait).times(2)
+
+    assert_raises(Freno::Throttler::WaitedTooLong) do
+      throttler.throttle(:mysqla) do
+        block_called = true
+      end
+    end
+
+    refute block_called, "block should not have been called"
+  end
 end
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@ -6,7 +6,7 @@ require "freno/throttler"
 require "minitest/autorun"
 require "mocha/minitest"

-class Freno::Client::Test < Minitest::Test
+class ClientTest < Minitest::Test
  def stubbed_faraday(&block)
    stubs = Faraday::Adapter::Test::Stubs.new(&block)
    Faraday.new do |builder|
@ -23,7 +23,7 @@ class Freno::Client::Test < Minitest::Test
  end
 end

-class Freno::Throttler::Test < Minitest::Test
+class ThrottlerTest < Minitest::Test
  def sample_client(faraday: nil)
    Freno::Client.new(faraday) do |freno|
      freno.default_store_type = :mysql