-
Notifications
You must be signed in to change notification settings - Fork 7.4k
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
Conversation
There was a problem hiding this 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.
Closes keycloak#38104 Signed-off-by: Václav Muzikář <vmuzikar@redhat.com>
- id: update-hosts | ||
name: Update /etc/hosts | ||
uses: ./.github/actions/update-hosts |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this 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!
There was a problem hiding this 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?
Closes keycloak#38104 Signed-off-by: Václav Muzikář <vmuzikar@redhat.com> (cherry picked from commit 8885a62)
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.