Conversation
All the queue methods are restricted to a generic type T that should inherit from ILightMessage. First, this is the same as asking that the type be an ILightMessage. However, upon inspecting the method one can see that there's no reason for the restriction - the methods can work with any objects.
bruno-brant
force-pushed
the
feat/add-unit-tests-for-lightworker
branch
from
f4899e4 to
f5eca94
Compare
bruno-brant
changed the title
|
Added a few more tests, now we have good coverage of the code in LightWorker. Hacked around a bit (exposed a few private methods that should be called by derived classes independently, for instance) which helped me achieve a good amount of coverage. There's a lot of code here that need refactoring and a lot of bad design choices that in later versions we need to remove. |
| [InlineData("anything")] | ||
| [InlineData(null)] | ||
| [InlineData(1)] | ||
| public void InvokeProcessWhenMethodIsNullReturnsExpectedValue(object message) |
There was a problem hiding this comment.
This does say it covers the InvokeProcess method when the method parameter is null, but it is not null.
Perhaps I misinterpreted the intention of this test?
| } | ||
|
|
||
| [Fact] | ||
| public void InvokeProcessWhenMethodIsEchoMessageIsEmptyReturnsNull() |
There was a problem hiding this comment.
I think there's something wrong with the name of this method: "InvokeProcess, when method is echo message is empty, returns null". Maybe I just didn't get it. Could you enlighten me, please?
Was it supposed to be something like "InvokeProcessWhenMethodIsEchoMethodAndMessageIsEmptyReturnsNull"?
|
|
||
| var message = ToJsonByteStream("anything"); | ||
|
|
||
| message[0] = _fixture.Create<byte>(); |
There was a problem hiding this comment.
Since MethodsCollection.EchoMethod expects an instance of Foobar as parameter, this line isn't necessary, because "anything" is not a valid serialized Foobar already. Is there any reason you're randomizing the first byte?
|
|
||
| var message = ToJsonByteStream("anything"); | ||
|
|
||
| message[0] = _fixture.Create<byte>(); |
There was a problem hiding this comment.
Kind of the same thing here. Is this randomization really necessary? In this case, you're testing the branch where the method does not have any parameters (line 142):
https://github.com/Avanade/Liquid-Application-Framework/blob/4f76069eb609ad20a570af4d44060b1a925782b3/src/Liquid.Activation/Worker/LightWorker.cs#L134-L150
In this case, we're never deserializing the message parameter (InvokeProcess will only deserialize when the method has one or more parameters), so it does not make any difference whether it is invalid or not.
There was a problem hiding this comment.
If this happens to be true and the "invalid message creation" part of the code is really not needed, then this test can be safely removed because InvokeProcessWhenMethodHasZeroParametersDoesntParseMessage already does this.
| sut.ValidateInput(viewModel); | ||
|
|
||
| // ACT & ASSERT | ||
| Assert.ThrowsAny<Exception>(() => sut.Factory<MockDomain>()); |
There was a problem hiding this comment.
I was wondering if we could check whether InvalidInputException is being thrown here, instead of Exception. What do you think?
|
|
||
| // TODO: There are translation errors in the Critics class:( | ||
| [Fact] | ||
| public void TerminateWhenCriticsHandlerHasCritics() |
There was a problem hiding this comment.
I think you missed the expected behavior part. Shouldn't it be "TerminateWhenCriticsHandlerHasCriticsThrows"?
2 participants
Add some unit tests for
LightWorker.Should be approved after #204.