RFR: 8273507: Convert test/jdk/java/nio/channels/Channels/TransferTo.java to TestNG test [v2]

[Markus KARG]

On Thu, 9 Sep 2021 14:13:14 GMT, Lance Andersen <lancea at openjdk.org> wrote:

> > > I would add a comment for for each DataProvider/Test to give future maintainers an overview of the intent of the methods
> > 
> > 
> > I support the general idea of making things more comprehensible, but I will abstain from adding comments in _this_ PR due to three reasons:
> > 
> > * This PR _solely_ has the target of technically introducing **TestNG** and I really do not want to mix concerns like technical merits with documental merits.
> > * The current PR is a blocker in the _critical path_ of other, higher prioritized PRs. We should try not to slow it down _unnecessarily_.
> > * I dislike code comments and would instead prefer to turn this source code into **BDD** style, i. e. changing the test name into "shouldDoThisInThatCase" style and introducing "given", "when" and "then" sections inside the tests. As this source code is a copy of two other original files, I will do it for all three files in one shot to keep their inherent relationship.
> > 
> > So in the best sense of KISS, SoC and "release early, release often", it makes sense that I will create an (or mutiple) explicit, subsequent PR(s) for introducing BDD instead of adding code comments in the current (slow and big) PR.
> 
> We are going to have to agree to disagree here. Comments are/should be part of the development process including for tests. There is no reason to create additional issues or new PRs given the tests is already being modified.
> 
> > > I would add a comment for for each DataProvider/Test to give future maintainers an overview of the intent of the methods
> > 
> > 
> > I support the general idea of making things more comprehensible, but I will abstain from adding comments in _this_ PR due to three reasons:
> > 
> > * This PR _solely_ has the target of technically introducing **TestNG** and I really do not want to mix concerns like technical merits with documental merits.
> > * The current PR is a blocker in the _critical path_ of other, higher prioritized PRs. We should try not to slow it down _unnecessarily_.
> > * I dislike code comments and would instead prefer to turn this source code into **BDD** style, i. e. changing the test name into "shouldDoThisInThatCase" style and introducing "given", "when" and "then" sections inside the tests. As this source code is a copy of two other original files, I will do it for all three files in one shot to keep their inherent relationship.
> > 
> > So in the best sense of KISS, SoC and "release early, release often", it makes sense that I will create an (or mutiple) explicit, subsequent PR(s) for introducing BDD instead of adding code comments in the current (slow and big) PR.
> 
> We are going to agree to disagree on this. There is no reason to create additional issues/PRs for improving the readability/documentation. Comments do not have to be extensive but enough to introduce the intent of the test. Now is the right time to add comments given the test is being updated.
> 
> As we evolve/maintain OpenJDK, we want to keep maintainability for current and future developers in mind. Adding documentation/comments while we are updating/adding to the project will help in this endeavor (and I know it would have helped me while navigating areas of the codebase I was less familiar with).

So this effectively means that you rather want to *prevent* this PR, which was demanded by @AlanBateman, and rather want to live with the test *as-is*, unless I add more comments that are *totally unrelated* with this PR? So what if I just don't agree here? Then you will never get *either* the comments *nor* will Alan get TestNG.

-------------

PR: https://git.openjdk.java.net/jdk/pull/5421