Skip to content

Run CI for master branch push and PRs.#2811

Draft
simi wants to merge 2 commits into
masterfrom
modernize-ci
Draft

Run CI for master branch push and PRs.#2811
simi wants to merge 2 commits into
masterfrom
modernize-ci

Conversation

@simi

@simi simi commented Jun 8, 2025

Copy link
Copy Markdown
Contributor

No description provided.

@voxik

voxik commented Jun 19, 2025

Copy link
Copy Markdown
Contributor

Looking into the CI failures, specifically to rack_3 (3.2, spec_rack), I believe this is due to sinatra/sinatra@cd3e00d introduced by (Sinatra /) Rack::Protection 4.1.0, where the www.example.com is not on the list of enabled hosts.

@simi

simi commented Jun 19, 2025

Copy link
Copy Markdown
Contributor Author

Thanks @voxik, I seen also some date related issues. Locally CI looks much better. I'll keep exploring, feel free to share patches (or open PR to merge into this branch) to fix any test you're aware of why is failing.

@simi

simi commented Jun 19, 2025

Copy link
Copy Markdown
Contributor Author

Any idea what to use instead of example.com or better to re-configure to enable?

@voxik

voxik commented Jun 19, 2025

Copy link
Copy Markdown
Contributor

Any idea what to use instead of example.com or better to re-configure to enable?

diff --git a/lib/capybara/spec/test_app.rb b/lib/capybara/spec/test_app.rb
index b29a1523..eabb2a8e 100644
--- a/lib/capybara/spec/test_app.rb
+++ b/lib/capybara/spec/test_app.rb
@@ -19,6 +19,7 @@ class TestApp < Sinatra::Base
   set :static, true
   set :raise_errors, true
   set :show_exceptions, false
+  set :host_authorization, { permitted_hosts: [] }
 
   # Also check lib/capybara/spec/views/*.erb for pages not listed here
 

This is the most simple workaround I have been able to come up with (working on PR right now). This just disables the check.

Or the check can be more specific, e.g. set :host_authorization, { permitted_hosts: [".example.com", "127.0.0.1"] }.

Just FTR, the 127.0.0.1 is there to pass the test in https://github.com/teamcapybara/capybara/blob/master/lib/capybara/spec/session/current_url_spec.rb while it seems weird to me 🤷

Rack::Protection 4.1.0 has introduced new
`Rack::Protection::HostAuthorization` [[1]]. This makes `www.example.com`
host to be rejected. The easiest solution is to disable this check for
the TestApp.

[1]: sinatra/sinatra#2053
@voxik

voxik commented Jun 19, 2025

Copy link
Copy Markdown
Contributor

Already looks better 👏 At least something is passing 🥳

@voxik

voxik commented Jun 19, 2025

Copy link
Copy Markdown
Contributor

This is likely issue for the rack_smoke (3.0, cucumber)

@simi

simi commented Jun 19, 2025

Copy link
Copy Markdown
Contributor Author

Ahh, I think we can remove 3.0. WDYT? Or worth to keep?

@voxik

voxik commented Jun 19, 2025

Copy link
Copy Markdown
Contributor

Ahh, I think we can remove 3.0. WDYT? Or worth to keep?

For me personally (and looking into this due to Fedora), I don't mind removing the 3.0 support. Nevertheless I can't say the xpath code would be more readable with just & instead of &block, so I wonder if the change is worth of it.

However, wearing my Red Hat fedora, RHEL users would likely not mind longer support for Ruby 3.0 which is default in RHEL 9 and will be supported for years. But I don't think I can stop the train (after all, RHEL 10 is out fresh) 😉

@simi

simi commented Jun 19, 2025

Copy link
Copy Markdown
Contributor Author

OK, I'll revert in xpath for now. I just blindly followed rubocop in there.

@simi

simi commented Jul 16, 2025

Copy link
Copy Markdown
Contributor Author

@voxik revert is not that simple, since it doesn't work with latest JRuby. I'll find out how to fix and report back.

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