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

Enhance StartDevCommandDistTest to detect problems with URI scheme on Windows #29593

Merged
merged 1 commit into from
May 22, 2024

Conversation

Pepo48
Copy link
Contributor

@Pepo48 Pepo48 commented May 16, 2024

  • added a test - a Windows drive letter within URI can cause issues

Related-to: #29329

Signed-off-by: Peter Zaoral pzaoral@redhat.com

Note:
#29329 was fixed earlier in #25333.

Copy link
Contributor

@vmuzikar vmuzikar left a comment

Choose a reason for hiding this comment

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

@Pepo48 Thank you for the fix! Could you please add some test for it?

@Pepo48 Pepo48 changed the title config-keystore does not work on Windows when absolute path is specified Enhance LoggingDistTest to detect problems with URI scheme on Windows May 22, 2024
@Pepo48
Copy link
Contributor Author

Pepo48 commented May 22, 2024

@vmuzikar this should be ready for a review.

Thanks!

Copy link
Contributor

@vmuzikar vmuzikar left a comment

Choose a reason for hiding this comment

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

@Pepo48 Thank you for adding the test (the actual fix is no longer needed).

The test LGTM but I'm afraid we don't run it on Win in the CI. We could probably move the test to a test class that runs with Win.

… Windows

* added a test - a Windows drive letter within URI can cause issues

Related-to: keycloak#29329

Signed-off-by: Peter Zaoral <pzaoral@redhat.com>
@Pepo48 Pepo48 changed the title Enhance LoggingDistTest to detect problems with URI scheme on Windows Enhance StartDevCommandDistTest to detect problems with URI scheme on Windows May 22, 2024
@Pepo48
Copy link
Contributor Author

Pepo48 commented May 22, 2024

Sorry @vmuzikar, I forgot about that. I placed it to the StartDevCommandDistTest as I feel it's logically closest. Let me know if it can stay like that. Thanks.

Copy link
Contributor

@vmuzikar vmuzikar left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the update! :)

@vmuzikar vmuzikar enabled auto-merge (squash) May 22, 2024 16:18
@vmuzikar vmuzikar merged commit bb12847 into keycloak:main May 22, 2024
66 checks passed
tmorin pushed a commit to tmorin/keycloak that referenced this pull request May 23, 2024
… Windows (keycloak#29593)

* added a test - a Windows drive letter within URI can cause issues

Related-to: keycloak#29329

Signed-off-by: Peter Zaoral <pzaoral@redhat.com>
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.

None yet

2 participants