rebus-org / Rebus.TestHelpers

:bus: Test helpers for Rebus (i.e. fake bus, saga fixture, etc.)
https://mookid.dk/category/rebus
Other
4 stars 2 forks source link

Possible issue with the 'PrepareConflict' #10

Closed mmdevterm closed 1 year ago

mmdevterm commented 1 year ago

Hello, while trying to unit test the saga conflict resolution I encountered a strange behavior. Could you please help me to clarify? This behavior is also reproducible with the unit test provided in this repository, therefore no custom code is needed.

TestSagaFixture_ResolveConflicts.CanSimulateConflict()

The saga ConflictSagaHandler provides a ResolveConflict method but this method is never called when calling the mentioned test. Saga flow is undisturbed and all the assertions pass.

From what I gather the ResolveConflict method should be called, or am I missing something?

mookid8000 commented 1 year ago

I just ran the test and verified that ResolveConflict is indeed called as expected.

How do you determine whether it's called?

mmdevterm commented 1 year ago

In order to verify it I have temporarily added an Exception to the ResolveConflict method. If I understand it correctly the Exception should be unhandled and the test should fail but it passes, which means that the method is never called.

2022-09-08_11h57_13 2022-09-08_11h59_28
mookid8000 commented 1 year ago

When I

throw new Exception("🤠");

in ResolveConflict like you suggest, I get this result:

image

mmdevterm commented 1 year ago

Since the very beginning I am referring to the test method named CanSimulateConflict() (the very first one in the file, not the MultipleConflicts). Which I belive is also marked as green on your screenshot.

If this might help to clarify confusion: adding another call of the Deliver causes the ResolveConflicts to be called in the discussed test:

2022-09-08_12h16_31
mookid8000 commented 1 year ago

Ah ok now I understand – thanks you for your persistence 😅

There's something about the design of this PrepareConflict method I need to think about. Honestly, I don't understand how it works, and therefore it's pretty hard for me to debug it 😁 but there's no doubt that it's just simulating conflicts, it doesn't actually experience them..

I would say that, in order to be able to simulate a conflict, one would have to provide a conflicting saga data instance, and not just specify a predicate. The conflicting saga data instance should then be the one passed to the ResolveConflict method.

I'll just play around with this a bit and see how it goes.

mookid8000 commented 1 year ago

OK I think it turned out pretty nice! So nice that I already released a new version, which I believe is much better!

Check it out – with the new version you help with staging the conflicting data, e.g. like so:

fixture.PrepareConflict<ConflictSagaHandlerData>(
    predicate: data => data.CorrelationId == "some-id",
    createConflictingSagaData: data => data.ReceivedStrings.Add("CONFLICT1")
);

(in this case preparing a conflicting saga instance by mutating the already existing saga instance that mathes the given predicate, adding the string "CONFLICT1" to its string collection)

thus making it easy to verify that ResolveConflict merges saga states as it should.

It's on NuGet.org as Rebus.TestHelpers 7.2.0 now.

Thanks for your patience in making be realize that there was an issue 👍

mmdevterm commented 1 year ago

Awesome! This indeed makes preparing conflict more "natural" like you stated in the comment and greatly reduces confusion about the "double-call" that I mentioned before.

Thank you for you time fixing this and for quick releasing the improvement! 🍻

Thanks for your patience in making be realize that there was an issue

no worries 😀