|
From: Mike C. <mik...@gm...> - 2007-05-29 18:36:22
|
Hi Steve,
Not dead, it just seems that way... I had a few reports from QA on
Friday that needed looking at, and a bank holiday yesterday.
I'm splitting your patch into smaller, more specific commits so that
it's easier for people to see what's changing and why. The ordering
bugfix is already committed, and I'm looking at the event-raising
feature now. (The MockObjectFactory will go last since it'll probably
be the most contentious.)
A few questions/comments:
* MockObject.RemoveEventHandler was only removing handlers if it
didn't contain them. Can I assume this was a simple copy-and-paste-o,
rather than anything more subtle?
* The signature for IMockObject.RaiseEvent looks a bit fuzzier than it
needs to be. We do very little with events here, so I'm not too
informed on the subject, but my NET 1.1 docs say that an event handler
delegate must have a signature of the form "void Foo(object sender,
EventArgs e)". Should RaiseEvent take one argument e, of type
(assignable to) EventArgs, and pass (this, e) to each handler? Or have
things been relaxed in NET 2.0?
* Not convinced that MockObject.RaiseEvent should throw if no handlers
have been attached. This isn't the behaviour of 'real' events.
* The patch's Fire.Event(name).On(mock).With(args) follows the same
syntactic pattern as Expect or Stub. I'm not sure we need that same
level of flexibility, particularly if the (object, EventArgs) rule for
handler delegates is still in force, and the With clause is
potentially misleading since it's not doing matching. Also, this
pattern only allows events to be raised out of a clear blue sky. Would
it be also be useful to support events firing in response to a mocked
method call? If so, how about implementing it as an IAction instead?
F'rinstance:
// Out of a clear blue sky
new EventAction("Beep", beepArgs).RaiseOn(myMock); // or add
syntactic sugar to taste
// Triggered by method call
Expect.Once.On(myMock)
.Method("Foo").WithAnyArguments
.Will(Raise.Event("Beep", beepArgs), Return.Value("something"));
Looks like a straightforward tweak, and it composes nicely with other actions.
* I'm going to need your nmock2.key at some point; it wasn't in the
patch or zip. I'm pretty sure that's the last of the missing files,
though.
cheers
Mike
|