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

[BUG] Add tracks attr_reader to Configuration #664

Merged
merged 1 commit into from
Jun 24, 2024

Conversation

alexsmartens
Copy link
Contributor

@alexsmartens alexsmartens commented Jun 4, 2024

Description:
This pull request adds the missing tracks attribute reader to the Rack::Attack::Configuration class. This change ensures that the tracks attribute is accessible, consistent with other attributes such as safelists, blocklists, and throttles.

How this was discovered?

> Rack::Attack.tracks.clear
=> warning: #<Class:Rack::Attack>#tracks at /.rbenv/versions/3.1.3/lib/ruby/3.1.0/forwardable.rb:157 forwarding to private method Rack::Attack::Configuration#tracks
/.rbenv/versions/3.1.3/lib/ruby/3.1.0/forwardable.rb:236:in `tracks': undefined method `tracks' for #<Rack::Attack::Configuration:0x000000010c09e978...

Changes:

  • Added :tracks to the attr_reader in lib/rack/attack/configuration.rb.
  • Updated spec/configuration_spec.rb to include tests for the tracks attribute alongside safelists, blocklists, and throttles.

Testing:

  • Verified that all existing tests pass.
  • Added new tests to check the accessibility of the tracks attribute.

This update enhances the completeness and consistency of the Rack::Attack::Configuration class, ensuring that all attributes are properly exposed and tested.

@alexsmartens alexsmartens changed the title Add tracks attr_reader to Configuration [BUG] Add tracks attr_reader to Configuration Jun 4, 2024
@alexsmartens
Copy link
Contributor Author

@santib can you please re-trigger the workflows, they were failing because how I required spec_helper. I've updated that so we should be good

spec/configuration_spec.rb Outdated Show resolved Hide resolved
@alexsmartens
Copy link
Contributor Author

@santib revised

Co-authored-by: Santiago Bartesaghi <santib@hey.com>

 # minitest instead of rspec
@alexsmartens
Copy link
Contributor Author

@santib the workflows failed because I used RSpec matchers. I revised the spec to use minitest

@santib santib merged commit 427fdfa into rack:main Jun 24, 2024
119 checks passed
@santib
Copy link
Collaborator

santib commented Jun 24, 2024

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants