Amazing! I claim it.

you got it! now amaze me ;-)

oops, I forgot to change status

Some things for simplifying method rule tests.

Example use is in NewLineLiteralTest2.cs

Waiting for comments and implementing some other stuff I'll show ya later.

P.S. To JB: delegates are supported too ;P

MethodRuleTest.cs
in xml comments "method" is sometimes spelled "metod"

NewLineLiteralTest2.cs:
public delegate string Blah ();
this is unused

looking at it, How are you supposed to call the delegate versions of 
AssertFailure
and AssertSuccess? I can't seem to do so without a rather ugly cast in my test 
method:
[Test]
public void HasEmpty() {
    AssertSuccess((Blah)GetEmpty);//after moving GetEmpty into the test fixture
}

With 2 delegates inside MethodRuleTest:
protected delegate void NoReturnMethod();
protected delegate object ReturnMethod();

and changing/adding overloads to AssertSuccess/AssertFailure:
protected void AssertSuccess(ReturnMethod method) {
    Assert.IsNull(Test(method));
}
protected void AssertSuccess(NoReturnMethod method) {
    Assert.IsNull(Test(method));
}
protected void AssertFailure(NoReturnMethod method) {
    Assert.IsNotNull(Test(method));
}
protected void AssertFailure(ReturnMethod method) {
    Assert.IsNotNull(Test(method));
}

the unit tests could now be written:
[Test]
public void HasEmpty() {
    AssertSuccess(GetEmpty);
}

Am I wrong here?

Another thing that would be helpful, in the sense of getting more test cases 
cheaply,
would be to provide some "standard" test cases. 

E.g. it's common for rules to check IL and every such rule must check if the 
analyzed
method has IL (p/invoke wouldn't). Right now we _need_ every rule to add it's 
own
p/invoke declaration to test this (which doesn't happen). However providing a 
pool
of, few but *well* selected, methods (and types) would make it easier. This 
should
simplify the testing of "PASS" tests and help get our unit test coverage higher 
(but
I must said I'm impressed by the current % ;-)

Daniel, please also look at comment in this thread
http://groups.google.com/group/gendarme/browse_thread/thread/7601b1f4384c3a19
Thanks!

Added more and more overloads. All tested so don't worry :-)
Usage samples I attach show both using delegates stuff where 
possible/convenient and
strings in other cases. Some overloads are still unused but I think as we begin
porting  old tests / writing new ones, they will be very handy :) .
Again, waiting for your comments & suggestions.

Cheers,
Dan :p

P.S. More helper methods a-la GetPInvokeMethod are coming, it's just I have to 
go to
sleep now, and I've written it a minute ago.

P.P.S. I wonder why NUnit doesn't want to load generic types containing PInvoke
declarations - had to move one into another class (which does not look _that_ 
good, I
suppose).

I've just noticed you added another comment - sorry :-) !
I'll make desired changes tommorrow.

I have much work to do, so I think I will have it done next day. Sorry for 
delays.

Hey, don't be sorry, I can't keep up reviewing ;-)
Take the time you need (due date is 22th :-) it's much funnier this way!

Some new stuff (e.g. generating simple MethodDefinition's on the fly if 
requested),
all code to extend is covered with documentation, getting types/methods from 
other
assemblies, etc.
Not everything is covered with example use cases, but I am sure that as we 
develop
new rules (currently only method rules are supported, type rule base fixture is
coming later), we will be able to use more of this code, thus not having to 
spend
much time at writing/debugging/modifying boring-and-duplicating-everywhere 
code, and
focus on creativity instead :-)

Like I said earlier I like this very much, in particular the end result (the 
test
themselves), but I do have some small comments ;-) 

* since we inherit from a base class, e.g. MethodRuleTest, could we not avoid 
having
TestHelper ? because right now it seems a mix of nunit 1.x (requires a common 
base
class) and 2.x (no base class, Assert helper class). I don't mind much which 
one (for
Gendarme, NUnit decision made a lot of sense) but I'm not sure we gain much by 
having
both.

* we should avoid calls like those 
    Assert.IsNotNull (messages);
    Assert.AreEqual (expectedCount, messages.Count);
(e.g. in AssertRuleFailure) where no message (string) is supplied to NUnit's 
Assert
method calls. This makes things difficult to debug and, since we already have a 
lot
of information at hands, it's easy to fix :)
  We should be able to have multiple calls to AssertRule[Success|Failure|...] inside
the same method without any confusion (or depending on the exception/assertion 
line #).

* Things like AssertRuleSuccessFor[GeneratedMethod|ExternalMethod] seems a bit 
too
specialized (for my taste ;-), and could lead to a *lot* of methods to handle 
every
cases (even more with the API changes introducing more than Success and 
Failure).
Having a single (or few) API to call an (unbound) numbers of possible "perfect" 
case
should be enough to ease high-coverage of the rules.

E.g. having a namespace that exclude all common types... that are known not to 
break
any "standard" rule (or updated not to)
namespace Perfect {
   interface IInterfaceTestCase {}
   enum EnumTestCase {}
   struct StructTestCase {}
   ...
}
and accessing them (via a enum or something more fancy ;-), e.g.
   AssertRuleSuccess (PerfectTypes.Interface);
would ensure the test is successful when called on a "perfect" interface - and 
be
easy to extend without adding new methods (and tests and documentation...). This
could become a standard item in rule test fixture.

[Test]
public void StuffTheRuleDoesNotHandle ()
{
   // this would ensure higher coverage, because we tend not to test stuff
   // that the rule does not, or should not, handle (and we could miss some)
   AssertRuleDoesNotApply (PerfectTypes.Interface);
   AssertRuleDoesNotApply (PerfectTypes.Enum);
   AssertRuleDoesNotApply (PerfectTypes.Struct);
   AssertRuleDoesNotApply (PerfectMethods.PInvoke);
   AssertRuleDoesNotApply (PerfectMethods.Abstract);
}

* The current API seems huge, at least when compared to Gendarme API. We 
*might* have
to see which API we like more and remove (or comment) the one that aren't 
popular.
Rule simplicity will not exists if writing the tests *seems* complicated.

* How do you propose we use this new code ? Personally I don't like the idea of
referencing NUnit assemblies from Gendarme.Framework (since it make NUnit a 
dependency).
  - referencing the source code from each project ? (may be hard with some build tools)
  - a new assembly for testing ? Gendarme.*.dll ?


Last note: I'm not gonna commit this in SVN right away because, like a said a 
few
times, we are planning API changes that we'll need to be reflected in this (and 
in
unit tests) - and I don't want to "rework" the test 2 times within a month ;-)

But, if everything is ready, I think we should move our current tests to this 
model
(with any updates before then) when doing the API upgrade (because I don't want 
to
"rework" the test afterward either ;-)

Great work!

I see it the way there will be a Test.Rules assembly referenced in all Test.* 
projects.

> since we inherit from a base class, e.g. MethodRuleTest, could we not avoid 
having
TestHelper ? 

Nope, it will be reused in TypeRuleTest. However we can define the very base 
RuleTest<T> class with those helper methods and then make 
(Type|Method)RuleTest<T> 
inherit it (I like this even better).

> we should avoid calls like those 
    Assert.IsNotNull (messages);
    Assert.AreEqual (expectedCount, messages.Count);
(e.g. in AssertRuleFailure) where no message (string) is supplied to NUnit's 
Assert
method calls

Sure. I'll fix it this evening.

> Having a single (or few) API to call an (unbound) numbers of possible 
"perfect" 
case should be enough to ease high-coverage of the rules.

Well, you're right. Now that I look again, they really seem to be too 
specialized.
You know, first I've been thinking about something very like that (for method 
rules):

AssertRuleSuccess (CommonMethods.ExternalMethod);
AssertRuleSuccess (CommonMethods.AbstractMethod);

However, in most rules it will not do, because in most cases we must also think 
of 
access modifiers and so on, so such "simplification" becomes even a possible 
source 
of mistakes.

So I think I will just implement AssertRuleDoesNotApply (PerfectMethods) where 
PerfectMethods is an enum with only one value - ExternalMethod, and then we'll 
add 
kind of methods which are commonly used.

Speaking of types, I'll add AssertRuleDoesNotApply (PerfectTypes) for the ones 
you've written in previous post.

> We *might* have to see which API we like more and remove (or comment) the one 
that 
aren't popular.

Sure, but we'll see it only after at least several tests are ported.

> Great work!

Thanks,
Dan.

> I see it the way there will be a Test.Rules assembly ...

Great. This is also what I think is best :)

> However we can define the very base RuleTest<T> class with those helper 
methods

yes, that's what I had in mind ;-)

> However, in most rules it will not do,

Right. But I have mixed feelings here. It may be easier (and more readable) to 
define
such cases (even if there's a bit of duplication) inside the test fixture than, 
let's
say, reading all the doc to find the right candidate. It's also more bullet 
proof
against changes in the Test assembly. The enum based [method|type]-list would 
be only
(well mostly) for very basic cases, where rules are not being (but should) 
tested
right now.

> I'll add AssertRuleDoesNotApply

that was an example using the new proposed API (the one where MessageCollection 
dies
;-), it doesn't match anything currently. You better implement both 
AssertRuleSuccess
and AssertRuleFailure today.

> Sure, but we'll see it only after at least several tests are ported.

Exactly! that's why this needs to be used a bit more before committing. The API
update will be a great test to determine what's useful (saves time) or not.

Here is updated version.
Code duplication (especially overloads) is pretty much reduced, and code now is 
lots
easier to read/maintain.
Now MethodRuleTest has an ancestor, RuleTest, from which TypeRuleTest derives 
(again,
not implemented yet). Removed TestHelper class and those Generate methods (will 
be
replaced by Common(Types|Methods|Etc) stuff soon).
Everything is covered by documentation.

No new things, just a new look at old ones - and I think this look looks a lot 
better ;-)

>> However we can define the very base RuleTest<T> class with those helper 
methods

>yes, that's what I had in mind ;-)

Yes, me too.  I agree :)

About the PerfectTypes and CommonMethods, I think it's a good idea because a 
lot of
times we are mocking these types and methods, and we rewrite them continuously. 
 If
we achieve a way to check these canonical scenarios, we could remove a lot of 
code
duplicated in the tests.

About the code distribution, I prefer put it in an external assembly.  This is
because referencing for each project can be a hard task because we are 
maintaining 2
build tools (MonoDevelop and Autotools).  Then, we will have other assembly 
depending
on Gendarme and NUnit, and we can create a gendarme-framework-test.pc file for 
use
pkg-config.

I'm really impressed with the end result.  Great work guys !

New update!
I've added TypeRuleTest<TRule> and simplified some other stuff.
Also, added support for Perfect(Methods|Types).

0 bytes is kinda separator between tests and common files (Test.Rules.dll).

Well I asked for it so here it goes... I'm amazed :)

Now I can't wait for the API changes to get a chance to use it in SVN :)

Thanks a bunch, you did a great job!
Sebastien