yreynhout / AggregateSource

Lightweight infrastructure for doing eventsourcing using aggregates
BSD 3-Clause "New" or "Revised" License
257 stars 60 forks source link

Add separate AggregateSource.Testing.NUnit package #12

Closed jen20 closed 11 years ago

jen20 commented 11 years ago

To make a super-duper-happy-path from NuGet, I've separated out the NUnitExtensions for AggregateSource.Testing from the sample and moved them into their own library, since it seems that this is something every project is going to need.

Will also do a version targetting xUnit.net for those who are that way inclined (I assume no-one uses MSTest, though may be wrong there!)

Summary of tasks completed below:

All tested up until the "push" phase, and the packages with dependencies work correctly from a local NuGet server.

yreynhout commented 11 years ago

While I agree this will be something every project will need, I have some objections to the current incarnation. Let me explain ...

  1. This way I'm coupling myself to the cadence of the unit testing fx in addition to whatever other dependency I have plus my own cadence, something I'd rather not do. I don't consider it healthy in the long run. There are ways around this, and I think FluentAssertions has gotten this "right". It does dynamic discovery of the unit testing fx in place (with an order of precedence if multiple are found) and leverages the only thing we really need, i.e. the assert exception of the unit testing fx at hand.
  2. There would be "a lot" (I know, that's pretty relative in this case) of duplication between the various unit testing fxs with the current incarnation. As said, the only thing we really need is the unit testing fx's assert exception. There's an opportunity here to share a lot of the "running a test specification", where the only point of integration with a unit testing fx is the use of its assert exception if the result is not as expected. The result of a run needs to be modelled explicitly for constructor, factory, command & query specifications (both event centric as well as exception centric).
  3. It's making the assumption that authors of messages will have overridden the equals method and implemented it for the purpose of testing structural equality. Not only is this tedious, but there are various alternative ways to do this kind of structural equality testing. I'd like a message author to be able to decide for himself how he's going to go about. That doesn't mean we couldn't provide some out of the box.
  4. It's incomplete (and no, that's not your fault). I discovered this with the collaborative aggregate testing (for which we'll never be able to implement this kind of package BTW) a while back. While the specification may be event centric, it could very well be that an exception is thrown, or the other way around, the specification may be exception centric, but events get produced (right now, this just barfs the exception). The Exception-/EventCentricTestResult classes are a testimony to this. Again, another reason why I'd rather have explicit results.
  5. Customization of test specification/result human output is something I value as well. Again, this is about not making any assumptions (such as ToString() being overridden on messages) and extensibility. That doesn't mean we couldn't provide something out of the box, but it needs to be pluggable.

I hope you understand why I'm not inclined to pull this in as is. If you really feel we need this "now", I'm willing to consider it as a "contribution" with limited lifespan, with no guarantee that it will work the same way in the future. Since people might be writing lots of specs and specs are the "gold" of one's software, I'd rather get this as "right" as humanly possible the first time.

Regards, Yves.

yreynhout commented 11 years ago

For a gist of my current thinking ... https://gist.github.com/yreynhout/6103143

yreynhout commented 11 years ago

Fixed as of 1b3c35a432b79fa0223dde810db40bc1b4cff18b