Closed lmvittori closed 6 years ago
Thanks for the contribution, very appreciated! Unfortunately I can't approve it and I have to ask you important changes. The approach for these tests is not quite right. If not mistaken, these are unit tests for a service located in the service layer. All the dependencies you have here for the service should be mocked (unit test vs integration). So for this particular case, you don't have to insert records in the database (even considering that it's an in-memory db). You have to mock your dependency to achieve the scenarios you want to test. For mocking we are using this library.
I didn't realized before, but you're modifying a class where tests are wrong too. We should fix those too. We need to mock all the dependencies.
@lmvittori going to merge your work because I want to add more stuff! I slightly fixed minor things what were making build fail and I have to remove a test because I couldn't make it work quickly. I plan to review it in detail later. Thanks again!!!
You should put a description like this on your PRs
Background
<Explain what was the problem, what decisions you took to solve them.>
Changes done
<Enumerate changes done, that are not obvious from the contents of the code. Explain only general approaches to the changes. Explain design decisions if any.>
Pending to be done
<Enumerate known pending tasks and why they weren't addressed here.>
Notes