Ewan10 / Task-Manager

0 stars 0 forks source link

Add unit tests #5

Open thompsy opened 2 years ago

thompsy commented 2 years ago

Currently there are no unit tests. We should definitely be doing some testing.

You may need to think about refactoring some of your methods to make them easier to test but we can talk about that in person. In the mean-time here's some reasonable discussion about some high-level design principles that can make code easier to test: https://softwareengineering.stackexchange.com/questions/153410/what-are-the-design-principles-that-promote-testable-code-designing-testable-c

Ewan10 commented 2 years ago

Yes those principles SOLID are presented in Urma's, Warburton's book. I printed out some other notes you sent some months ago about testing from Baeldung site or so and I read them. I let you arrange a meetup to discuss how to write tests for the public API as I can see multiple ways and some may not conform to the formal testing standards. I thought about refactoring the original methods but I can think of other ways which might be inappropriate.

thompsy commented 2 years ago

Once you've finished that current PR you could start on this. The TaskManager will be factored in a way that makes it easier to test. There are a number of things we could test:

In each case a sensible approach would be to have a separate test for each of these cases. Each test case should probably have an input file and an output file that live the src/test/resources/ folder. Each test case can then create a TaskManager class, load the input file, take some action e.g. update the status of a task, save the output file to a temporary location and compare it with the expected output file.

Ewan10 commented 2 years ago

I created a folder tree test/java and test/resources. Inside test/java I created a class for the tests. I imported the dependency junit.jupiter and I wrote three methods for three of the four tests above. And for one method I wrote part of the code. Regarding the test I need to see examples of code in order to be able to translate descriptions of tests to code. This is why I didn't manage to write the third method from the above list and part of the first method. I put inside the method body some comments of what I should do but don't know yet how to code it. After creating the appropriate repos I can push for you to review the already written code and give advice.

Also again I 'm waiting for a confirmation for the same issue of sqlite as backend whether for each remote I should create a new local repo where I will make commits from. I forgot and I already wrote the tests above while being inside the refactor-user-input.

Ewan10 commented 2 years ago

Andrew after I merged the PR the additions of the test code are available in the main branch with the test folder and you can open the TaskManagerTest.java and see where I stuck. I don't know how to translate some tests to code. If you could have a look?

thompsy commented 2 years ago

Ok, so two quick things here.

Firstly, you should never merge code straight into the main branch. All code that goes into the main branch should start in it's own separate branch and then be reviewed in a PR before being merged. A PR is the place where we should be discussing these questions rather than here as it's much harder to comment on specific lines of code here.

Secondly, can you outline here precisely what tests you're having trouble with. What is the test name? What are you trying to test? What will the inputs be? What will the expected outputs be?

Ewan10 commented 2 years ago

Just above you specified the tests for me to implement:

  1. the standard happy flow of loading a file and saving it and comparing that the two files are identical
  2. the happy case where we update the status of a task and check we save it correctly
  3. the unhappy case where the input file that we're loading is badly formatted or has duplicate ids.
  4. the unhappy case where the file doesn't exist

So inside main you will find a test folder which I uploaded for the first time so it is not for this time hard to track the new code. In test/java it is the only file TaskManagerTest.java There is where I implemented the tests by test methods. It is one class the TaskManagerTest.java and it is implemented one method per test inside there. Inside the method body where I didn't implement the code I wrote comments of what should be done but I find it uncertain how to code it. This is for test 3 and test 1.

How to check if the file is badly formatted? If it has duplicate ids I need to write a method that accesses and compares the id's of the tasks in the map. Should I add this method in the TaskManager.java class? As I need to access fields of tasks. I see that I need to create methods and extend the interface for accessing fields to perform the tests and Idon't know where to define them.

Regarding test 1 you can see the comments inside the body of the method in TaskManagerTest.java `What are the exact objects in assertEquals() to be compared?

thompsy commented 2 years ago

So, writing these tests should serve as a prompt for you to think further about the functionality of the program that you're writing and how it should behave under certain circumstances.

So, for example, how does it handle a badly formatted file? There are a number of ways a file could be badly formatted. It could have duplicate ids, it could be invalid json, it could be empty, it could be full of garbage data. How should your program handle each of these cases? How would it know? Would it just throw an exception, or would it read in the data and try and use it?

In terms of the first test and what you should be comparing, perhaps the best way to think about this is to think about what state the TaskManager is holding. This is basically just the tasks map and the id. You should probably compare both of these to expected values to make sure the state is correct. Whether you do that in a loop or something else isn't hugely important at this stage. The key thing is that your code is correct and easy to understand.

Ewan10 commented 2 years ago

Andrew don't you have a look to the tests in add-unit-tests branch and make some comments?