RFR: 8273507: Convert test/jdk/java/nio/channels/Channels/TransferTo.java to TestNG test [v2]
Markus KARG
github.com+1701815+mkarg at openjdk.java.net
Sun Sep 26 15:30:59 UTC 2021
On Mon, 13 Sep 2021 09:10:13 GMT, Alan Bateman <alanb at openjdk.org> wrote:
>>> > * 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.
>>
>> BTW, if some day someone needs to undo my PR, then all the BDD changes will be gone, too. This is ridiculuous. Due to the mass of changes BDD imposes, this **must** be separate PRs to prevent that.
>>
>> And a final word: Alain and Brian both agreed that the original test is fine, maintainable and comprehensive. If you take a minute to read the actual code being tested, you will notice that the test already is self-explanatory already without any comments.
>
>> * 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.
>
> Ugh, please no. As Lance said, we need the source and tests to be as easy to maintain as possible. In this case it would be helpful to have comments on testStreamContents and checkTransferredContents so that it's very quick for someone to know what the test is trying to when it fails (in this area, most of the effort on tests is spent chasing intermittent test failures).
@AlanBateman wrote:
> In this case it would be helpful to have comments on testStreamContents and checkTransferredContents so that it's very quick for someone to know what the test is trying to when it fails (in this area, most of the effort on tests is spent chasing intermittent test failures).
Done by https://github.com/openjdk/jdk/pull/5421/commits/0f0569843faa7c319bb03990eb922406ac78e626.
-------------
PR: https://git.openjdk.java.net/jdk/pull/5421
More information about the nio-dev
mailing list