ashmeet-kandhari commented on PR #20991:
URL: https://github.com/apache/flink/pull/20991#issuecomment-1345624195
> hi @ashmeet-kandhari thanks for conflicts resolution I left some comments
(i didn't go through the whole changeset) however there are several types of
things which would be nice to address
>
> 1. Since junit5 classes and tests method could be package private (may be
except some cases with hierarchies). So would nice to have it fixed
> 2. Using the `META-INF/services/org.junit.jupiter.api.extension.Extension`
is the better approach here than using annotation
`@ExtendWith(TestLoggerExtension.class)` for every class.
> 3. in junit5 timeout unit by default is seconds while in junit4 it is
millis. It looks like this should be taken into account or may be better set it
explicitly like `@Timeout(value = 30_000, unit = TimeUnit.MILLISECONDS)`
> 4. I noticed there are tests failing would be nice to understand if it is
result of changes within this PR or something else
> 5. GitHub still shows changes for `docs/themes/book` in this PR. It should
be reverted since it has nothing to do with kafka-connector to junit5 migration
Hi @snuyanzin
I have addressed the first 3 points from the above comments.
Regarding the rest of the comments
* _4th Comment:_ The from what I could see the tests failed for 2 reasons
1. Compilation failed and suggestion was to run `mvn spotless:apply`.
But I always run it before committing it. I did test now with the new changes
made locally and the tests run fine. So we can see if this comes up again
2. `docs_404_checks` stage failed with the error mentioned below. Not
sure what this means as I haven't changed anything there
> Error: Error building site:
"/home/vsts/work/1/s/docs/content.zh/_index.md:36:1": failed to extract
shortcode: template for shortcode "columns" not found
* _5th Comment:_ I might need some help on how to revert the change for
that. As I cannot see anything in intellij that points to changes made in that
folder
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
