8000 Hardcode nip.io test hosts to `/etc/hosts` by vmuzikar · Pull Request #39044 · keycloak/keycloak · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Hardcode nip.io test hosts to /etc/hosts #39044

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

Merged
merged 1 commit into from
Apr 24, 2025
Merged

Conversation

vmuzikar
Copy link
Contributor
@vmuzikar vmuzikar commented Apr 17, 2025

Closes #38104

Partially reverts 9eb336a to be on the safe side that real hostnames are used.

The hostnames are right now manually managed in the GH Action. We could of course automate this but greping the whole code base for nip.io hosts before each CI run seemed like too much overhead and unnecessary overengineering. It might not also capture all hostnames as some are programmatically specified in the tests – so it would require human interaction (to avoid stuff like this) anyway. Given it's just a handful of hostnames I went for the manual approach. Let me know if that works for you.

A test run of 2x100 DefaultCookieProviderTest seems stable.

Copy link
Contributor
@mabartos mabartos left a comment

Choose a reason for hiding this comment

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

@vmuzikar I'm ok with this approach.

The action needs to be fixed for Win, though.

@vmuzikar vmuzikar marked this pull request as draft April 17, 2025 08:37
Closes keycloak#38104

Signed-off-by: Václav Muzikář <vmuzikar@redhat.com>
@vmuzikar vmuzikar marked this pull request as ready for review April 17, 2025 11:07
@vmuzikar vmuzikar requested a review from mabartos April 17, 2025 11:07
Comment on lines +17 to +19
- id: update-hosts
name: Update /etc/hosts
uses: ./.github/actions/update-hosts
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure if we need to use this somewhere else too in the UI/JS tests? CC @jonkoops

Copy link
Contributor

Choose a reason for hiding this comment

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

No, those should be fine without.

@vmuzikar vmuzikar requested a review from Pepo48 April 17, 2025 11:10
Copy link
Contributor
@mabartos mabartos left a comment

Choose a reason for hiding this comment

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

@vmuzikar LGTM from my PoV, thanks!

Copy link
Contributor
@mhajas mhajas left a comment

Choose a reason for hiding this comment

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

Thanks for the change @vmuzikar! The PR LGTM. Do we wait for other reviews, or can we merge?

@vmuzikar vmuzikar merged commit 8885a62 into keycloak:main Apr 24, 2025
81 checks passed
vmuzikar added a commit to vmuzikar/keycloak that referenced this pull request Apr 24, 2025
Closes keycloak#38104

Signed-off-by: Václav Muzikář <vmuzikar@redhat.com>
(cherry picked from commit 8885a62)
@vmuzikar vmuzikar deleted the nip.io branch April 24, 2025 07:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Temporary failure in name resolution with nip.io
5 participants
0