slfan1989 commented on PR #4349: URL: https://github.com/apache/hadoop/pull/4349#issuecomment-1150485882
> 1. leave assertEquals alone. I don't care whether it's deprecated or not -it is not going to be removed. > 2. new tests should use AssertJ's assertThat for anything complex...it's a great design and we are adopting it broadly. > 3. This patch is getting dangerously close to the "create backport pain for no reason" class of patch. Just because something is mark as deprecated doesn't mean we have to worry, not unless they're going to remove it completely. If that happens on a JUnit version upgrade we can deal with it then. > 4. And if the plan really is to move off deprecated JUnit, the switch MUST be to assertJ; there is so little use of Junit assertThat that it is isolated. > > Overall: I am really unsure whether we really want to do this. It's a minor cleanup that creates pain. If you really want to go ahead with it then > > 1. move to assertj > 2. do it in a module by module basis. > 3. Remember that any change which goes near hadoop-aws and hadoop-azure what is required to be tested against the endpoint of that organisation. Thank you very much for your suggestion, it helped me a lot. From a personal point of view, I still hope that the old deprecated method can be replaced by a new method, which can make the code better. 1. I will replace the deprecated Junit assertThat with assertJ assertThat. 2. Modifications to imports will be minimal. 3. I will repair it module by module. Thanks again!!! -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
