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.

Sign up for GitHub

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

Jump to bottom

Use latest version of rack-test #1801

Merged
merged 1 commit into from
Jul 25, 2022
Merged

Use latest version of rack-test #1801

merged 1 commit into from
Jul 25, 2022

Conversation

epergo
Copy link
Member

@epergo epergo commented Jul 25, 2022

Allow versions greater (and equals) than 2.0 for the rack-test gem. The only change needed was to remove the build_rack_test_session in test_helper because it uses the deprecated rack_mock_session which calls build_rack_test_session again creating an infinite loop

epergo
epergo commented Jul 25, 2022
View reviewed changes
sinatra.gemspec
@@ -46,4 +46,6 @@ EOF
s.add_dependency 'tilt', '~> 2.0'
s.add_dependency 'rack-protection', version
s.add_dependency 'mustermann', '~> 3.0'

s.add_development_dependency 'rack-test', '~> 2'
Copy link
Member Author

@epergo epergo Jul 25, 2022 •
edited

Choose a reason for hiding this comment

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

I've moved the dependency here so all projects define it in the same place

@jkowens
Copy link
Member

jkowens commented Jul 25, 2022

@epergo did you see @jeremyevans' comment about the default_env method?

@epergo
Copy link
Member Author

epergo commented Jul 25, 2022 •
edited

@epergo did you see @jeremyevans' comment about the default_env method?

😅 Nope, didn't see it. The second issue would need to be addressed. I'm having a look and in all tests global_env is empty, do you know if it is used somehow? It looks like that it isn't and the custom Session class could be dropped altogether.

@epergo
Copy link
Member Author

epergo commented Jul 25, 2022

I've removed the Session custom class so we use the one provided by rack-test because we weren't using the extra functionality. Also I've added the last version of rack-test from the repository to check if there is any other failure. All steps are working

@jkowens jkowens merged commit 9ab6a9f into sinatra:master Jul 25, 2022
@jkowens
Copy link
Member

jkowens commented Jul 25, 2022

Looks good, thanks!

@jkowens jkowens mentioned this pull request Jul 25, 2022
Closed
@epergo epergo deleted the ep/update-rack-test branch July 25, 2022 16:35
dentarg added a commit to dentarg/sinatra that referenced this pull request Dec 22, 2023
@dentarg
CI: use latest rack-test release, not trunk
90774fd 
This comes from sinatra#1801 and maybe
it was a reason back then but I don't see any now.

This just makes things simpler, for example, the Docker Ruby images
based on Alpine does not come with git.
@dentarg dentarg mentioned this pull request Dec 22, 2023
Merged
dentarg added a commit that referenced this pull request Dec 22, 2023
@dentarg
CI: use latest rack-test release, not trunk (#1969)
11119a8 
This comes from #1801 and maybe
it was a reason back then but I don't see any now.

This just makes things simpler, for example, the Docker Ruby images
based on Alpine does not come with git.
@rondales
Copy link

Allow versions greater (and equals) than 2.0 for the rack-test gem. The only change needed was to remove the build_rack_test_

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@epergo @jkowens @rondales