On 25 October 2006 at 18:38, "Denis Kishenko" <[EMAIL PROTECTED]> wrote: > > Mark, I have just tried your tool. It's really helpful, thanks a lot! > > It's so pitty that script doesn't fix issues by itself =)
It could (and I have been known to use scripts to fix things) but as Nathan recently pointed out. It's not always a good idea. Specifically, my tool is not perfect at identifying the different types of assertEquals calls (e.g. the variants for double where the last argument might look like a constant but isn't the expected value but a delta). It does a reasonable job but without implementing a full parser it's never going to be 100% reliable. I did use a script - a one-off on the command line - to fix a significant number of assertEquals calls to use assertTrue/assertFalse/assertNull a week or so ago, but I did also scan the diff to see if it looked sane. Scanning the diff was almost as tedious as fixing them manually. ;-( Regards, Mark. > 2006/10/25, Geir Magnusson Jr. <[EMAIL PROTECTED]>: > > > > > > Mark Hindess wrote: > > > On 25 October 2006 at 7:41, "Geir Magnusson Jr." <[EMAIL PROTECTED]> > > > wrote: > > >> Cool - but why not just put into SVN somewhere? > > > > > > Okay. classlib/trunk/support/tools/bin perhaps? > > > > Sure. Whatever you feel is best. I have no strong opinion. We do have > > junit tests in DRLVM too, but we can "reach over" and use from there for > > now. > > > > geir > > > > > > > > -Mark. > > > > > >> either in enhanced/tools or classlib/trunk somewhere where it can be > > >> invoked as an option by people from ant (so that we can wire it into the > > >> CI system...) > > >> > > >> geir > > >> > > >> > > >> Mark Hindess wrote: > > >>> Earlier in the year we discussed junit best practice. For example, > > >>> making sure assertEquals calls have the expected and actual arguments i > n > > >>> the correct order to avoid getting confusing failure messages. > > >>> > > >>> Robert posted a script a week or so ago, to look for some of junit > > >>> issues but it didn't handle asserts that spanned multiple lines so, > > >>> unfortunately, it was missing the majority of them. I had a script tha > t > > >>> I'd thrown together one evening that would handle multi-line asserts bu > t > > >>> annoyingly (because it read the whole file at once) couldn't report the > > >>> line number of the potential issue as Robert's script did. > > >>> > > >>> Inspired by Robert's post, I looked at my script again. I've now fixed > > >>> it to report line numbers, added a little bit of documentation and > > >>> attached it to a JIRA: > > >>> > > >>> https://issues.apache.org/jira/browse/HARMONY-1960 > > >>> > > >>> It finds quite a lot of potential problems (I've appended a summary of > > >>> the findings below). (There will be a few false positives but hopefull > y > > >>> not too many.) It would be nice to fix these issues - I fixed several > > >>> hundred while testing the script - but more importantly we should make > > >>> sure we avoid adding any new issues. > > >>> > > >>> Improvements to the script would be most welcome. > > >>> > > >>> Regards, > > >>> Mark. > > >>> > > >>> Types of issue identified > > >>> > > >>> 4949 should possibly use assertEquals > > >>> 815 actual may be a constant > > >>> 437 consider using separate asserts for each '&&' component > > >>> 330 exception may be left to junit > > >>> 135 actual *may* be a constant > > >>> 48 should be fail (always false) > > >>> 40 should be fail (always true) > > >>> 20 expected is null - should use assertNull > > >>> 12 consider using separate asserts for each '||' component > > >>> 8 expected is false - should use assertFalse > > >>> 7 expected is true - should use assertTrue > > >>> 1 should use assertNotNull > > >>> > > >>> > > >>> Number of Issues by module > > >>> > > >>> 1907 luni > > >>> 1440 swing > > >>> 699 math > > >>> 611 security > > >>> 335 text > > >>> 322 awt > > >>> 222 sound > > >>> 186 nio > > >>> 178 jndi > > >>> 123 archive > > >>> 118 auth > > >>> 117 crypto > > >>> 116 logging > > >>> 91 nio_char > > >>> 87 print > > >>> 74 regex > > >>> 68 concurrent > > >>> 45 beans > > >>> 41 x-net > > >>> 21 sql > > >>> 1 rmi > > >>> > > >>> > > >>> > > >>> > > > > > > > > > > > > > > -- > Denis M. Kishenko > Intel Middleware Products Division >