On 13/11/17 12:26, Konstantin Kolinko wrote: > 2017-11-13 13:42 GMT+03:00 Mark Thomas <ma...@apache.org>: >> On 13/11/17 09:49, Mark Thomas wrote: >> >> <snip/> >> >>> Hmm. >>> >>> As I started to look at this I realised that a large number of the >>> classes with errors are only using the static import a few times. Some >>> of them are using a mix of static and non-static imports. >>> >>> Given that switching to the non-static usage also fixes the issue, I'm >>> going to apply that fix first and then see what is left. Generally, I >>> plan to apply it when switching to the non-static usage results in >>> roughly the same amount of code or less. >> >> Given that: >> >> - the majority of the test classes don't use static imports >> - the majority of the test classes Gump is complaining about only use a >> few static Asserts >> - only a small number of test classes use a large number of static >> Asserts >> - global search and replace can remove all the static Assert imports >> quickly and easily >> - removing static Asserts fixes the Checkstyle warnings for Gump and >> works with all branches >> >> I'm going to go with removing static Assert (and similar) imports from >> the unit tests. >> > > Honestly, I do not like this,
Fair enough. I don't think it is perfect either. > but this is not a technical issue, it is your itch, not worth to spend > much time discussing. Also agreed. On small positive (for me at least) is that over the years I've thought about fixing the inconsistent use of static and non-static imports in the unit tests and this change does at least address that. > I think this reduces code readability, may surprise newcomers. (My > theory. One should ask a real newcomer.) Probably for a few tests - although overall I think the difference it makes is marginal. > I think the trends in writing asserts are to use assertThat + Hamcrest > library matcher, and they look better with static imports. Agreed and agreed. The unit tests we have now are a lot better than what we have before (i.e. not very much) but there is definitely a lot of scope fopr improving them, cleaning them up, reducing duplication, using better style, increasing coverage etc. Mark > https://github.com/junit-team/junit4/wiki/matchers-and-assertthat > > Best regards, > Konstantin Kolinko > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
