Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Specifications and type for failure reasons #113

Closed
malclear opened this issue May 4, 2019 · 12 comments
Closed

Specifications and type for failure reasons #113

malclear opened this issue May 4, 2019 · 12 comments

Comments

@malclear
Copy link
Contributor

malclear commented May 4, 2019

Hi Lutando,
I'm seeing that the "errors" parameter for the FailedExecutionResult has changed from an IEnumerable<string> to string[]. However, the type returned from WhyIsNotSatisfiedBy(T obj) in the Specification<T> class remains IEnumerable<string>. It is requiring that we now add ".ToArray( )" to our calls.
Was this intentional?

... new FailedExecutionResult(ourSpecification.WhyIsNotSatisfiedBy(command).ToArray());

Not a major issue, I think.

@Lutando
Copy link
Collaborator

Lutando commented May 4, 2019

Youre right, this is a breaking change that probably should not have happened. I'll make FailedExecutionResult support both and will will allocate memory for failure reasons there, its probably nicer if we allow it to also take in an IEnumerable and allocate in the ctor of FailedExecutionResult. My apologies for this 😅

@malclear
Copy link
Contributor Author

malclear commented May 4, 2019

No worries! I'm happy to help where I can.
In fact, before your next release, if you'd like me to build and test the Master branch against my codebase, I'd be glad to. Just let me know when you're about ready.

@malclear
Copy link
Contributor Author

malclear commented May 6, 2019

To append to this issue, and I've not figured out why it's happening, but in our unit tests if we use the ExpectMsg pattern, it fails with a timeout (even though I can see the aggregate code send the message): ExpectMsg<FailedExecutionResult>( ... ). However, if we use the Ask pattern for the call, it works! myManager.Ask<FailedExecutionResult>( ... )

This type of failure only happens with the latest 0.4.3 build.

@Lutando
Copy link
Collaborator

Lutando commented May 6, 2019

So in other words when you use a Tell(...) prior to ExpectMsg<FailedExecutionResult>( ... ) it doesn't work but when you use Ask<FailedExecutionResult> prior to ExpectMsg<FailedExecutionResult>( ... ) it does work?

@malclear
Copy link
Contributor Author

malclear commented May 6, 2019

Not quite... the first part of what you said is correct, but for my resolution, when I use Ask, I assign the result to a variable, ala: var result = await myManager.Ask<FailedExecutionResult>(myCommand), then that works.

@Lutando
Copy link
Collaborator

Lutando commented May 6, 2019

This smells like a quirk of akka.net's test kit. on the top of my head I'm not sure why the declaration and assignment would result in a positive test to be honest.

@Lutando
Copy link
Collaborator

Lutando commented May 6, 2019

Maybe I can find out why tomorrow 😁

@malclear
Copy link
Contributor Author

malclear commented May 6, 2019

ExpectMsg still works for the SuccessExecutionResult. It's only the FailedExecutionResult that requires the special handling. We can work around it, so no rush.

@Lutando
Copy link
Collaborator

Lutando commented May 7, 2019

very odd. Your previous comment actually makes me a little bit more confused.

I can see this working :

var commandProbe = CreateTestProbe("command-probe");
aggregateManager.Tell(command,commandProbe); <--- set probe as  sender
commandProbe.ExpectMsg<SuccessExecutionResult>();

or this :

var result = await aggregateManager.Ask<SuccessExecutionResult>(command); 
// no expect message pattern

but not this

var commandProbe = CreateTestProbe("command-probe");
var result = await aggregateManager.Ask<SuccessExecutionResult>(command); 
commandProbe.ExpectMsg<SuccessExecutionResult>();

@malclear
Copy link
Contributor Author

malclear commented May 7, 2019

It is the second, not the third or your examples (an Ask and an Expect would of course be redundant).
When we are testing for a failed Ask, we use your second example:
var result = await aggregateManager.Ask<FailedExecutionResult>(command); // no expect message pattern

The success responses can still be EXPECTed, but we have to ASK for the failed execution results. In other words, the return of a SuccessExecutionResult seems unchanged, but the return of FailedExecutionResult has begun failing with this release, given that we were using the ExpectMsg pattern. Personally, I suspect it has to do with the usage of params in the Ctor, but that's just a guess.

@Lutando
Copy link
Collaborator

Lutando commented May 7, 2019

You are right, Its serialization seems like it.

@Lutando
Copy link
Collaborator

Lutando commented May 15, 2019

this has been fixed in 0.4.5 :)

@Lutando Lutando closed this as completed May 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants