-
Notifications
You must be signed in to change notification settings - Fork 235
Add openfeign module #1180
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
base: main
Are you sure you want to change the base?
Add openfeign module #1180
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.
Good job! 👍
```xml | ||
<dependency> | ||
<groupId>io.qameta.allure</groupId> | ||
<artifactId>allure-open-feign</artifactId> |
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.
allure-openfeign
?
@BeforeAll | ||
static void setUp() { | ||
wireMockServer = new WireMockServer(options().dynamicPort()); | ||
wireMockServer.start(); |
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.
tear down?
import static com.github.tomakehurst.wiremock.client.WireMock.get; | ||
import static com.github.tomakehurst.wiremock.client.WireMock.urlEqualTo; | ||
import static com.github.tomakehurst.wiremock.core.WireMockConfiguration.options; | ||
import static org.junit.jupiter.api.Assertions.assertAll; |
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.
use AssertJ
assertions instead
|
||
assertAll( | ||
() -> assertEquals(new HelloWorldRecord("Hello World").getMessage(), helloWorldRecord.get().getMessage()), | ||
() -> assertTrue(attachmentNames.contains("Response"), "Cannot find attachment with name \"Response\""), |
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.
Don't use assertTrue
, AssertJ provides more convenient ways to check that
|
||
static WireMockServer wireMockServer; | ||
|
||
@BeforeAll |
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.
Since it's only a few tests, we can use @BeforeEach
instead?
} | ||
|
||
@Test | ||
void jsonBodyTest() { |
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.
Please, add tests for the plan body response and no response body
.getJsonHelloWorld()); | ||
}); | ||
|
||
List<String> attachmentNames = allureResults.getTestResults().stream() |
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.
check not only the attachment name, but the body as well
Hello! When writing additional tests, I discovered a nuance. If you need to use the feign.Response entity in an autotest, for example, to get the headers, the feign.codec.Decode#decode method is not called, which makes sense since the attachment is not attached. Using feign.ResponseInterceptor is not possible because the feign.Response that is passed to the feign.ResponseInterceptor#intercept method is designed so that you can only read the response body once. However, since openFeign supports multiple http clients, you can use the following construct:
I have tested it locally and it works both when the interface method returns feign.Response and when the DTO.If it makes sense, I can write tests for all the http clients that openFeign supports, and update the README with examples of how to use openFeign |
Add new module that automatically captures Feign client HTTP traffic as Allure attachments for comprehensive API test reporting.