TestCase.fail() bug

TestCase.fail() bug

[Chris Moller]

[-- Attachment #1: Type: text/plain, Size: 1530 bytes --]

Hello, Fryskers, wherever you are!

Per the attached exchange between Andrew and me, does anyone have any
idea how to fix this kind of bug?  I'm not a Java guy and haven't a
clue, and the suggestions I got from Phil on IRC regarding similar uses
of TestCase.fail() in frysk weren't similar enough to the uses in
AttachedSelf.java that I could apply them.

Any advice/suggestions/sympathy would be greatly appreciated,
Chris

-------- Original Message --------

Chris,

It's the new more pedantic ECJ; Mark's mentioned this on irc and 
frysk@.  I'd send it to frysk@ asking for suggestions :-)

Andrew

> Hey, Andrew,
>
> The following bug just popped up on some code you added on Friday.  It's
> coming from a rawhide machine, so I don't know if it's a real bug or a
> gcj/ecj gotcha.  Anyway, FWIW:
>
> /home/moller/tinkering/frysk/04-16/build/frysk-imports/../../frysk/frysk-imports/frysk/testbed/AttachedSelf.java:66:
> error: The static method fail(String) from the type Assert should be
> accessed directly
>         TestCase.fail (why);
>         ^^^^^^^^^^^^^^^^^^^
> /home/moller/tinkering/frysk/04-16/build/frysk-imports/../../frysk/frysk-imports/frysk/testbed/AttachedSelf.java:77:
> error: The static method fail(String) from the type Assert should be
> accessed directly
>         TestCase.fail ("unexpected signal " + sig);
>         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 2 problems (2 errors)
> make[3]: *** [frysk/sys/TestPtraceServer.o] Error 1
>
>
>
>
>   




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

TestCase.fail() bug

[Chris Moller]

[-- Attachment #1: Type: text/plain, Size: 1530 bytes --]

Hello, Fryskers, wherever you are!

Per the attached exchange between Andrew and me, does anyone have any
idea how to fix this kind of bug?  I'm not a Java guy and haven't a
clue, and the suggestions I got from Phil on IRC regarding similar uses
of TestCase.fail() in frysk weren't similar enough to the uses in
AttachedSelf.java that I could apply them.

Any advice/suggestions/sympathy would be greatly appreciated,
Chris

-------- Original Message --------

Chris,

It's the new more pedantic ECJ; Mark's mentioned this on irc and 
frysk@.  I'd send it to frysk@ asking for suggestions :-)

Andrew

> Hey, Andrew,
>
> The following bug just popped up on some code you added on Friday.  It's
> coming from a rawhide machine, so I don't know if it's a real bug or a
> gcj/ecj gotcha.  Anyway, FWIW:
>
> /home/moller/tinkering/frysk/04-16/build/frysk-imports/../../frysk/frysk-imports/frysk/testbed/AttachedSelf.java:66:
> error: The static method fail(String) from the type Assert should be
> accessed directly
>         TestCase.fail (why);
>         ^^^^^^^^^^^^^^^^^^^
> /home/moller/tinkering/frysk/04-16/build/frysk-imports/../../frysk/frysk-imports/frysk/testbed/AttachedSelf.java:77:
> error: The static method fail(String) from the type Assert should be
> accessed directly
>         TestCase.fail ("unexpected signal " + sig);
>         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 2 problems (2 errors)
> make[3]: *** [frysk/sys/TestPtraceServer.o] Error 1
>
>
>
>
>   




[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

Re: TestCase.fail() bug

[Andrew Cagney]

This is an example of ecj being just a bit to single minded about how 
the code should be written.  If the code is changed to use 
junit.Assert.assert* instead of frysk.junit.TestCase then we miss the 
opportunity change the behavior.   In fact, I'd like to suggest 
modifying frysk.junit.TestCase along these lines:

     public static void assertNull (String what, Object o)
    {
       Assert.assertNull(what,o);
       logger.log(Level.FINE, "assertNull {0} {1}\n", new Object[] 
{what, null});
    }
    public static void assertNull(Object o)
    {
       fail("called nameless assert method");
    }

the second would obviously take some cleanning up before it passed :-)

Andrew


Chris Moller wrote:
> Hello, Fryskers, wherever you are!
>
> Per the attached exchange between Andrew and me, does anyone have any
> idea how to fix this kind of bug?  I'm not a Java guy and haven't a
> clue, and the suggestions I got from Phil on IRC regarding similar uses
> of TestCase.fail() in frysk weren't similar enough to the uses in
> AttachedSelf.java that I could apply them.
>
> Any advice/suggestions/sympathy would be greatly appreciated,
> Chris
>
> -------- Original Message --------
>
> Chris,
>
> It's the new more pedantic ECJ; Mark's mentioned this on irc and 
> frysk@.  I'd send it to frysk@ asking for suggestions :-)
>
> Andrew
>
>   
>> Hey, Andrew,
>>
>> The following bug just popped up on some code you added on Friday.  It's
>> coming from a rawhide machine, so I don't know if it's a real bug or a
>> gcj/ecj gotcha.  Anyway, FWIW:
>>
>> /home/moller/tinkering/frysk/04-16/build/frysk-imports/../../frysk/frysk-imports/frysk/testbed/AttachedSelf.java:66:
>> error: The static method fail(String) from the type Assert should be
>> accessed directly
>>         TestCase.fail (why);
>>         ^^^^^^^^^^^^^^^^^^^
>> /home/moller/tinkering/frysk/04-16/build/frysk-imports/../../frysk/frysk-imports/frysk/testbed/AttachedSelf.java:77:
>> error: The static method fail(String) from the type Assert should be
>> accessed directly
>>         TestCase.fail ("unexpected signal " + sig);
>>         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>> 2 problems (2 errors)
>> make[3]: *** [frysk/sys/TestPtraceServer.o] Error 1
>>
>>
>>
>>
>>   
>>     
>
>
>
>   

Re: TestCase.fail() bug

[Chris Moller]

[-- Attachment #1: Type: text/plain, Size: 3139 bytes --]

I committed a change a while ago that adds

import junit.framework.Assert;

at the top and replaced the TestCase.fail(...)s with Assert.fail(...)s. 
I'm way too Java-impaired to know if my patch achieves anything like
your suggestion but, if not, I'll try to hack your stuff in.

BTW, I've discovered, so far, about 50 hyper-pedantic-gcj "problems" in
frysk.  I've committed patches for some of them and am working on the
rest.  Some are /real/ brain-dead, like "if (1 == 0) throw ....;" to fix
a problem where something was supposed to throw an exceeption but didn't
have any code to do so.  Unused parameters and redundant casts make up
most of the problems.

Chris

Andrew Cagney wrote:
> This is an example of ecj being just a bit to single minded about how
> the code should be written.  If the code is changed to use
> junit.Assert.assert* instead of frysk.junit.TestCase then we miss the
> opportunity change the behavior.   In fact, I'd like to suggest
> modifying frysk.junit.TestCase along these lines:
>
>     public static void assertNull (String what, Object o)
>    {
>       Assert.assertNull(what,o);
>       logger.log(Level.FINE, "assertNull {0} {1}\n", new Object[]
> {what, null});
>    }
>    public static void assertNull(Object o)
>    {
>       fail("called nameless assert method");
>    }
>
> the second would obviously take some cleanning up before it passed :-)
>
> Andrew
>
>
> Chris Moller wrote:
>> Hello, Fryskers, wherever you are!
>>
>> Per the attached exchange between Andrew and me, does anyone have any
>> idea how to fix this kind of bug?  I'm not a Java guy and haven't a
>> clue, and the suggestions I got from Phil on IRC regarding similar uses
>> of TestCase.fail() in frysk weren't similar enough to the uses in
>> AttachedSelf.java that I could apply them.
>>
>> Any advice/suggestions/sympathy would be greatly appreciated,
>> Chris
>>
>> -------- Original Message --------
>>
>> Chris,
>>
>> It's the new more pedantic ECJ; Mark's mentioned this on irc and
>> frysk@.  I'd send it to frysk@ asking for suggestions :-)
>>
>> Andrew
>>
>>  
>>> Hey, Andrew,
>>>
>>> The following bug just popped up on some code you added on Friday. 
>>> It's
>>> coming from a rawhide machine, so I don't know if it's a real bug or a
>>> gcj/ecj gotcha.  Anyway, FWIW:
>>>
>>> /home/moller/tinkering/frysk/04-16/build/frysk-imports/../../frysk/frysk-imports/frysk/testbed/AttachedSelf.java:66:
>>>
>>> error: The static method fail(String) from the type Assert should be
>>> accessed directly
>>>         TestCase.fail (why);
>>>         ^^^^^^^^^^^^^^^^^^^
>>> /home/moller/tinkering/frysk/04-16/build/frysk-imports/../../frysk/frysk-imports/frysk/testbed/AttachedSelf.java:77:
>>>
>>> error: The static method fail(String) from the type Assert should be
>>> accessed directly
>>>         TestCase.fail ("unexpected signal " + sig);
>>>         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
>>> 2 problems (2 errors)
>>> make[3]: *** [frysk/sys/TestPtraceServer.o] Error 1
>>>
>>>
>>>
>>>
>>>       
>>
>>
>>
>>   
>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

Re: TestCase.fail() bug

[Phil Muldoon]

Chris Moller wrote:
> BTW, I've discovered, so far, about 50 hyper-pedantic-gcj "problems" in
> frysk.  I've committed patches for some of them and am working on the
> rest.  Some are /real/ brain-dead, like "if (1 == 0) throw ....;" to fix
> a problem where something was supposed to throw an exceeption but didn't
> have any code to do so.  Unused parameters and redundant casts make up
> most of the problems.
>   


The declares a throw "Exception" but the code will never throw that 
"Exception", ever is a new ECJ warning I think. In that case, I'd delete 
the the Exception that was  declared in the throw statement itself 
instead of faking throwing it. It might unravel try/catches up the 
code-line too, but those would be bugs as well.

Regards

Phil

Re: TestCase.fail() bug

[Nurdin Premji]

Chris Moller wrote:
> I committed a change a while ago that adds
>
> import junit.framework.Assert;
>
> at the top and replaced the TestCase.fail(...)s with Assert.fail(...)s. 
> I'm way too Java-impaired to know if my patch achieves anything like
> your suggestion but, if not, I'll try to hack your stuff in.
>   
Should just be a matter of appending to frysk.junit.TestCase.java:
public static fail(String why) throws AssertionError.
{
super.fail(why)
}
or even:
{
throw new AssertionError(why);
}

I'm unsure of the fail method signature but I think that is correct.
Then we can go back to using TestCase.fail(why) and TestCase in general 
for everything (test related).

Re: TestCase.fail() bug

[Chris Moller]

[-- Attachment #1: Type: text/plain, Size: 1678 bytes --]



Phil Muldoon wrote:
> Chris Moller wrote:
>> BTW, I've discovered, so far, about 50 hyper-pedantic-gcj "problems" in
>> frysk.  I've committed patches for some of them and am working on the
>> rest.  Some are /real/ brain-dead, like "if (1 == 0) throw ....;" to fix
>> a problem where something was supposed to throw an exceeption but didn't
>> have any code to do so.  Unused parameters and redundant casts make up
>> most of the problems.
>>   
>
>
> The declares a throw "Exception" but the code will never throw that
> "Exception", ever is a new ECJ warning I think. In that case, I'd
> delete the the Exception that was  declared in the throw statement
> itself instead of faking throwing it. It might unravel try/catches up
> the code-line too, but those would be bugs as well.

What I'm trying to achieve is to make exactly zero functional change to
the way the code works.  Some of the "bugs" are in places where the code
is obviously incomplete (e.g., the unused parms in Match.java) and I
don't want to screw up the work-in-progress of whoever is writing the
code.  In the missing throw case (in
frysk-core/frysk/dom/DOMFactory.java) , I don't know if it's missing
because it's not needed, or because the code isn't complete, and I don't
want to have to deal with any possible repercussions (like gcj
complaining that createDOM doesn't specify it throws
NoDebugInfoException when that behaviour is expected elsewhere).

I'm documenting all my hacks and I'll send the lot to debug-list when
I'm done so you /real/ Java guys can, if you want to, go back and turn
my hacks into something sensible.

cm

>
> Regards
>
> Phil
>


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

Re: TestCase.fail() bug

[Tom Tromey]

>>>>> "Chris" == Chris Moller <cmoller@redhat.com> writes:

Chris> BTW, I've discovered, so far, about 50 hyper-pedantic-gcj
Chris> "problems" in frysk.

One option would be to disable the warnings you don't like.  ecj has
many knobs, and IMO some of the default warnings are not really that
useful.

Tom

Re: TestCase.fail() bug

[Chris Moller]

[-- Attachment #1: Type: text/plain, Size: 466 bytes --]

-Wno-unused helped a lot.  Now all we need is
-Wdont-sweat-the-small-stuff switch.

Tom Tromey wrote:
>>>>>> "Chris" == Chris Moller <cmoller@redhat.com> writes:
>>>>>>             
>
> Chris> BTW, I've discovered, so far, about 50 hyper-pedantic-gcj
> Chris> "problems" in frysk.
>
> One option would be to disable the warnings you don't like.  ecj has
> many knobs, and IMO some of the default warnings are not really that
> useful.
>
> Tom
>   


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 251 bytes --]

Re: TestCase.fail() bug

[Mark Wielaard]

On Wed, 2007-04-18 at 16:17 -0400, Chris Moller wrote:
> Tom Tromey wrote:
> >>>>>> "Chris" == Chris Moller <cmoller@redhat.com> writes:
> >>>>>>             
> >
> > Chris> BTW, I've discovered, so far, about 50 hyper-pedantic-gcj
> > Chris> "problems" in frysk.
> >
> > One option would be to disable the warnings you don't like.  ecj has
> > many knobs, and IMO some of the default warnings are not really that
> > useful.
>
> -Wno-unused helped a lot.  Now all we need is
> -Wdont-sweat-the-small-stuff switch.

I better strategy would be to drop the -Wall, see what warnings remain
and then add and/or disable specific warnings. For GNU Classpath we use
the default and then disable the following:
-warn:-deprecation,serial,typeHiding,unchecked,unused,varargsCast
Some of these warnings are only disabled because they cannot be avoided
when implementing the core libraries, so you might not want to disable
all of them, but it might be a good start.

Cheers,

Mark

Re: TestCase.fail() bug

[Andrew Cagney]

Mark, yes,

One of the more useful features of gcc is being able to cherry-pick the 
warnings.  For instance warn of unused locals, but not unused 
parameters.  Perhaps sun's front end is better at this?

Andrew

/me suffers a feeling of deja-vu

Mark Wielaard wrote:
>
> I better strategy would be to drop the -Wall, see what warnings remain
> and then add and/or disable specific warnings. For GNU Classpath we use
> the default and then disable the following:
> -warn:-deprecation,serial,typeHiding,unchecked,unused,varargsCast
> Some of these warnings are only disabled because they cannot be avoided
> when implementing the core libraries, so you might not want to disable
> all of them, but it might be a good start.
>
> Cheers,
>
> Mark
>
>   

Re: TestCase.fail() bug

[Mark Wielaard]

[-- Attachment #1: Type: text/plain, Size: 822 bytes --]

On Thu, 2007-04-19 at 09:43 -0400, Andrew Cagney wrote:
> One of the more useful features of gcc is being able to cherry-pick the 
> warnings.  For instance warn of unused locals, but not unused 
> parameters.

Yes that is actually what I propose. ecj can enable/disable specific
warnings. it is just that defaults (and frysk currently using -Wall)
enable some which are not always that useful. A list of all different
warnings can be found at:
http://help.eclipse.org/help31/index.jsp?topic=/org.eclipse.jdt.doc.isv/guide/jdt_api_compile.htm

> Perhaps sun's front end is better at this?

I haven't played with that yet much. But there is a summer of code
project to work on gcj support for it:
http://code.google.com/soc/gcc/appinfo.html?csaid=47DBA282F08CF
So we should know soon :)

Cheers,

Mark

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: TestCase.fail() bug

[Tom Tromey]

>> Perhaps sun's front end is better at this?

Mark> I haven't played with that yet much. But there is a summer of code
Mark> project to work on gcj support for it:
Mark> http://code.google.com/soc/gcc/appinfo.html?csaid=47DBA282F08CF
Mark> So we should know soon :)

My impression is that javac does not really have many warning
settings.  But, I admit I have not looked lately.

I believe we have 52 warning options for gcj+ecj.  There are plenty
addressing unused things:

    Wunused-local
    Wunused-argument
    Wunused-import
    Wunused-private
    Wunused-label
    Wunused-thrown
    Wunused

Hmmmm, I don't see docs on this though.
Anyway the algorithm is simple, take an ecj flag and turn it from
camelCaseStyle to gnu-hyphen-style.

Tom