-
Notifications
You must be signed in to change notification settings - Fork 375
Ensure backwards compatiblity #676
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR ensures backwards compatibility by running the test suite on an older OpenSSL gem version and updating related configurations.
- Implements a skip mechanism in tests when RSA-PSS support is missing
- Removes tests that raise errors for PS algorithms with newer OpenSSL versions
- Adjusts the gemfile, RuboCop configuration, and workflow Ruby version to support the legacy OpenSSL gem
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| spec/jwt/jwt_spec.rb | Adds skip checks for RSA-PSS support and removes error tests for PS algorithms |
| spec/jwt/jwa/ps_spec.rb | Adds skip check for RSA-PSS support in PS-specific tests |
| lib/jwt/error.rb | Removes the RequiredDependencyError, aligning with backwards compatibility changes |
| gemfiles/openssl.gemfile | Downgrades the OpenSSL gem version constraint to support older versions |
| .rubocop.yml | Disables the Naming/PredicateMethod rule to align with compatibility requirements |
| .github/workflows/test.yml | Downgrades the Ruby version for one test matrix from 3.0 to 2.5 to reflect compatibility needs |
Comments suppressed due to low confidence (4)
spec/jwt/jwt_spec.rb:204
- [nitpick] Consider adding a comment explaining why tests for PS algorithms in the error branch were removed, clarifying the intent for backwards compatibility.
if Gem::Version.new(OpenSSL::VERSION) >= Gem::Version.new('2.1')
lib/jwt/error.rb:7
- Verify that removing the RequiredDependencyError does not negatively affect error handling in parts of the code that rely on this exception.
class RequiredDependencyError < StandardError; end
gemfiles/openssl.gemfile:5
- Ensure that constraining the openssl gem to versions '< 2.0' aligns with the intended compatibility and does not inadvertently exclude required features.
gem "openssl", "< 2.0"
.github/workflows/test.yml:53
- Downgrading the Ruby version for this test matrix could impact compatibility and behavior of modern syntax features; please ensure the test suite comprehensively covers potential version-specific issues.
ruby: "2.5"
| EnforcedStyle: gemspec | ||
|
|
||
| Naming/PredicateMethod: | ||
| Enabled: false |
Copilot
AI
Jun 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Disabling the Naming/PredicateMethod rule might mask potential naming consistency issues; consider documenting the rationale for this change.
| Enabled: false | |
| Enabled: false # Disabled to allow flexibility in naming methods that do not strictly follow predicate conventions. |
The core gem was upgraded to version 3 a couple of weeks ago https://github.com/jwt/ruby-jwt/tree/v3.0.0. Removed the RequiredDependencyError constant as that has been removed in the gem itself as part of jwt/ruby-jwt#676
Description
Ensure we run the test suite on a older openssl gem version (2.0).
Checklist
Before the PR can be merged be sure the following are checked: