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

Add rails smoke tests to external tests. #2208

Merged
merged 1 commit into from
Jun 13, 2024
Merged

Conversation

ioquatix
Copy link
Member

We've had a couple of challenges relating to what essentially amount to compatibility issues between Rails and Rack, e.g. rails/rails#52066. Let's ensure that Rails' smoke tests all pass as part of Rack's CI. This should give earlier indications about potential issues.

Copy link
Contributor

@jeremyevans jeremyevans left a comment

Choose a reason for hiding this comment

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

May want to consider adding Sinatra as an external test as well. Sinatra and Rails probably cover 90% of Rack usage. But that can be done later.

@dentarg
Copy link
Contributor

dentarg commented Jun 12, 2024

I'm testing rack-head in the Sinatra repo. Currently there is one known failure: sinatra/sinatra#2014

@ioquatix
Copy link
Member Author

@dentarg are you able to create a PR to add Sinatra to the external tests?

@dentarg
Copy link
Contributor

dentarg commented Jun 12, 2024

I have too little time right now

@ioquatix
Copy link
Member Author

ioquatix commented Jun 12, 2024

Once rails/rails#52096 is merged (or the issue is otherwise resolved), we can merge this.

@simi
Copy link
Contributor

simi commented Jun 12, 2024

I have too little time right now

I have some. #2210

@ioquatix ioquatix merged commit 8270e43 into main Jun 13, 2024
23 of 29 checks passed
@ioquatix ioquatix deleted the rails-external-tests branch June 13, 2024 01:05
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.

None yet

4 participants