update(tests, failing): migrate tests in /failing to JUnit5 #4448
Conversation
|
@timo-a Note that @JooHyukKim is also submitting incremental conversion so it'd be good to try to avoid overlapping work. |
| import com.fasterxml.jackson.databind.testutil.DatabindTestUtil; | ||
| import org.junit.jupiter.api.Test; | ||
|
|
||
| import java.util.HashMap; |
There was a problem hiding this comment.
It's too bad import statements got reordered wrong way (against our coding style).
But I can fix these on my end.
There was a problem hiding this comment.
Sorry about that! I had used Intellij's "optimize imports" unaware that this project has a custom code style that differs from it. I think a first step could be a PR template pointing to this code style.
But ideally there should be an a GitHub Action formatting all PRs so no additional burden is placed on contributors (or reviewers). google-java-format would provide this functionality (and overrule your current code style) but you don't seem to be a fan 😄
I guess you could also define your own autoformatter but that looks like a lot of work...
There was a problem hiding this comment.
Yeah, I am not a fan of auto-formatters in general since I am not sure they can be configured in a way to match the style I actually want (f.ex regarding pretty comment blocks; or varying maximum line lengths) and end up achieving unity ("foolish consistency... hobgoblin of small minds" or however quote goes) but not optimal style.
At work auto-formatters are used and produce strictly hideous output wrt indentation and very verbose (statements split in unnecessarily many lines) so I have some experience.
As a bonus, CI fails if one forgets run mvn fmt:format which tends to happen. What's not to like.
But I digress.
OTOH, if there is a way to do only partial reformat for specific aspects/parts -- for example, sorting import statements -- that'd be nice. And to apply reformatting just to changed files.
This is probably something IDEs would be in better position to do; but they on the other hand cannot be forced on developers so cannot quite be automated.
There was a problem hiding this comment.
(sorry to hear about your work experience - it seems CI should just run mvn fmt:format, then devs don't need to remember)
I believe there is a way to automate format checks on PRs for jackson with openrewrite:
In addition to performing one time code changes (like the JUnit4->5 migration) openrewrite can be used to add suggestions to any PR. The suggestions are only made for lines in the change set of the PR. You can specify beforehand in the pom.xml which changes should be suggested and at review time you can apply suggestions with the click of a button, or choose to ignore them.
A reference example can be found here: https://github.com/langchain4j/langchain4j/pull/673/files (I would replace googleapis/code-suggester@v4 with reviewdog though).
This mechanism could be added to jackson right away and e.g. make some static code analysis suggestions on PRs.
Your code style could also be encoded in custom recipes. There is a template for that: https://github.com/moderneinc/rewrite-recipe-starter you'd just have to write the recipes and publish them on maven central.
I'm planning to publish my own recipes eventually and could write some for a jackson code style as well, if you're interested. Please let me know if you'd like to have such a mechanism for the jackson repositories.
There was a problem hiding this comment.
I am open to experimentation on things that add suggestions. As is often the case, I probably won't have time to drive these changes but would be happy to be the co-pilot. Many of best improvements have come about this way.
And in a few cases my concerns have proven unfounded; Dependabot has (f.ex) been net positive for updating CI and Maven plugin versions (but not so useful in regular deps, but less need for help there)
2 participants
No description provided.