Add Repository tests to document the expected behaviour

meteor-space / event-sourcing

CQRS and Event Sourcing Infrastructure for Meteor.

MIT License

63 stars 9 forks source link

Add Repository tests to document the expected behaviour #56

Closed rhyslbw closed 8 years ago

rhyslbw commented 8 years ago

Ok so this is quite simple, and it's only ~~slipped through~~ an issue as we had a gap in test coverage on the class. Space.eventSourcing.Repository#save() currently calls a non-existent instance method getCommands(), which the logic allowed without borking for some reason. This resulting in no error thrown and an empty array being persisted.

[x] Add tests
[ ] ~~add triggeringMessage field to commit as the metadata @darko-mijic has been expecting to be in commands~~

rhyslbw commented 8 years ago

Ah ok, this is actually not a bug, but a misunderstanding of what changes.commands is for. This array should hold the commands generated by Space.eventSourcing.Process, not the command that caused the state-change. I'll review and see what needs to be done to resolve this issue, may just be the inclusion to document the expectation

rhyslbw commented 8 years ago

I've decided to just use this PR to document the functionality in the new Repository test, and not add any features here to store the triggering message, as it's just added complexity at this point. The events are the source of truth, so it may be mis-direction by persisting this extra meta, not to mention the added bulk in each commit

DominikGuzei commented 8 years ago

Ok @darko-mijic, so the problem was that you misunderstood which commands are being saved into the commit :wink: its not the command that initiates the state change but the commands that are triggered from within the process! @rhyslbw thanks for the added test coverage, happy to see that this is no bug we overlooked after the latest changes :smiley: