Skip to content

Conversation

@jeremyevans
Copy link
Contributor

See individual commits for what is fixed by each. Briefly:

Bug Fixes:

Dead Code Removal or Deprecation:

Final two commits are adding nocov markings around code that cannot be hit with current Ruby versions, and just adding specs to get back to 100% coverage.

@jeremyevans jeremyevans requested a review from ioquatix July 5, 2024 21:22
This method was added in f80df39,
but only for Digest support.  Digest authentication was removed
in Rack 3.1, so there is no need for this method.

Found by coverage testing.
This is a private method, and not used since
3855d1d.

The reason for deprecation before removal is that the
method only exists so it can be overridden in subclasses
(d8ccbf3), so if subclasses
were overriding it and calling super, we need to warn them
about the removal.

Found by coverage testing.
The only call of this method was removed in
51b0c26.

This method was only added a couple years ago, when Digest support
was removed in 253fc5b (it previously
called Rack::Auth::Digest::Params::dequote). So it seems unlikely
there are external users, and therefore it should not need
deprecation.

Found by coverage testing.
…filenames

This regexp was already questionable because of the use of ^ and $.
It turns out this was no longer needed, since the parser earlier
in the same method handles the quotes, so the result is that this
was removing escaped quotes that were part of the filename.

Found by coverage testing.
Rack::MediaType#type handles empty string, since
1848851.  The change does
not appear to be deliberate, though.  An alternative approach
would be both of them raising an exception if provided an
empty string, but I think handling the empty string similar
to nil is friendlier.

Found by coverage testing.
Technically, this leaves 1 branch uncovered, but that is about
to be removed.
@ioquatix ioquatix force-pushed the coverage-testing-fixes branch from 8260867 to c8ff2bb Compare July 5, 2024 23:14
Copy link
Member

@ioquatix ioquatix left a comment

Choose a reason for hiding this comment

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

Thanks Jeremy, this looks good to me.

@jeremyevans jeremyevans merged commit 4af2d21 into rack:main Jul 5, 2024
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