Closed GoogleCodeExporter closed 9 years ago
Amazing! I claim it.
Original comment by dan.abra...@gmail.com
on 14 Jan 2008 at 4:08
you got it! now amaze me ;-)
Original comment by sebastie...@gmail.com
on 14 Jan 2008 at 4:19
oops, I forgot to change status
Original comment by sebastie...@gmail.com
on 14 Jan 2008 at 6:12
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
Original comment by dan.abra...@gmail.com
on 14 Jan 2008 at 10:02
Attachments:
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?
Original comment by after.fa...@gmail.com
on 14 Jan 2008 at 11:46
Attachments:
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 % ;-)
Original comment by sebastie...@gmail.com
on 15 Jan 2008 at 3:49
Daniel, please also look at comment in this thread
http://groups.google.com/group/gendarme/browse_thread/thread/7601b1f4384c3a19
Thanks!
Original comment by sebastie...@gmail.com
on 15 Jan 2008 at 8:12
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).
Original comment by dan.abra...@gmail.com
on 15 Jan 2008 at 8:54
Attachments:
I've just noticed you added another comment - sorry :-) !
I'll make desired changes tommorrow.
Original comment by dan.abra...@gmail.com
on 15 Jan 2008 at 8:56
I have much work to do, so I think I will have it done next day. Sorry for
delays.
Original comment by dan.abra...@gmail.com
on 16 Jan 2008 at 8:17
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!
Original comment by sebastie...@gmail.com
on 16 Jan 2008 at 8:34
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 :-)
Original comment by dan.abra...@gmail.com
on 17 Jan 2008 at 7:30
Attachments:
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!
Original comment by sebastie...@gmail.com
on 18 Jan 2008 at 12:55
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.
Original comment by dan.abra...@gmail.com
on 18 Jan 2008 at 11:29
> 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.
Original comment by sebastie...@gmail.com
on 18 Jan 2008 at 1:23
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 ;-)
Original comment by dan.abra...@gmail.com
on 18 Jan 2008 at 7:42
Attachments:
>> 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 !
Original comment by nestor.s...@gmail.com
on 18 Jan 2008 at 7:56
New update!
I've added TypeRuleTest<TRule> and simplified some other stuff.
Also, added support for Perfect(Methods|Types).
Original comment by dan.abra...@gmail.com
on 19 Jan 2008 at 6:30
Attachments:
0 bytes is kinda separator between tests and common files (Test.Rules.dll).
Original comment by dan.abra...@gmail.com
on 19 Jan 2008 at 6:31
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
Original comment by sebastie...@gmail.com
on 19 Jan 2008 at 6:49
Original issue reported on code.google.com by
sebastie...@gmail.com
on 14 Jan 2008 at 3:55