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

Convert jaxrs tests to java #12816

Merged
merged 4 commits into from
Dec 1, 2024
Merged

Convert jaxrs tests to java #12816

merged 4 commits into from
Dec 1, 2024

Conversation

laurit
Copy link
Contributor

@laurit laurit commented Nov 29, 2024

part of #7195

@laurit laurit marked this pull request as ready for review November 30, 2024 10:53
@laurit laurit requested a review from a team as a code owner November 30, 2024 10:53
protected void configure(HttpServerTestOptions options) {
super.configure(options);

options.setResponseCodeOnNonStandardHttpMethod(500);
Copy link
Member

Choose a reason for hiding this comment

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

the groovy test had a conditional to use 500 for testLatestDeps and 405 otherwise. I see you implemented that in the Jetty test, but that was not needed here as well? I assume not since the tests are all passing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is not needed because of using a lambda in

    serverFactory.setProvider(
        (ExceptionMapper<Exception>)
            exception ->
                Response.status(500)
                    .type(MediaType.TEXT_PLAIN_TYPE)
                    .entity(exception.getMessage())
                    .build());

When choosing an exception mapper cxf inspects the exception mapper classes and ends up choosing between a built in mapper and our mapper. When using regular class it picks the built in one and when using lambda our one for the non standard http method test.

Copy link
Member

Choose a reason for hiding this comment

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

thank you for the explanation

@trask trask merged commit 28db801 into open-telemetry:main Dec 1, 2024
56 checks passed
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.

3 participants