k0r0pt / Project-Tauro

A Router WiFi key recovery/cracking tool with a twist.

GNU General Public License v3.0

88 stars 16 forks source link

UpdaterDao Unittests #21

Closed katsadim closed 5 years ago

katsadim commented 5 years ago

What's this PR for?

Unit tests for UpdaterDao. This PR is the second part of resolving issue: k0r0pt/Project-Tauro#10

What to emphasize on when reviewing?

Now the DB connection is injected into UpdaterDao for easier mocking. No need to use fancy tools to mess around with the bytecode cause of the static method call DbConnEngine.getConnection().
I took the liberty of introducing Spock framework into the project. Spock will certainly make unit testing easier and fun for sure.
This will fail for now because there is a need to accept the Linked Pull Request.

Linked Pull Requests

Repo: k0r0pt/common_build_config#8

katsadim commented 5 years ago

@sudiptosarkar, trust me Spock is one of the best framework for writing unit tests. Groovy makes the unit tests much more readable and an absolute beauty. I say that you should give it a try.

The groovy gradle plugin should be applied first in order to run the test. If you accept the linked PR k0r0pt/common_build_config#8 then the spock tests will be enabled.

sumitsarkar commented 5 years ago

@katsadim I agree that Spock indeed is a very good framework for BDD. But the question is whether introducing a different JVM language just for the sake of introducing BDD makes sense for this project.

I don't see the RoI in introducing a new language and paradigm when the complexity of the project is not very big.

As for what you are trying to test... Since SQLite requires a single lock to write to the file system, you'd see that the method for insertion of row is synchronized. Taking your first idea further, we can simply inject the UpdaterDao into the scrapper implementations. That does exactly what you are looking for. As for testing UpdaterDao, the purpose is to test the generated PreparedStatement. The easy way would be to extract the logic of the preparation of the statement to a separate function and write simple JUnit tests for validating it.

Having said that, I took your idea and created #23. You can have a look at that.

katsadim commented 5 years ago

Imho, introducing a different language is not such a big deal, if it indeed makes testing easier and more enjoyable. Junit compared to Spock is a pain to write and leads to code repetition and really hard to understand tests. As a side note I have seen smaller projects using Spock!

I am glad you liked the dependency injection idea and you took a step even further.

Just a suggestion here would be to not refer to the concrete class when passing it around but rather declare an interface. This way, when we want for example to save a station in a different storage (DB, cloud, microservice, you name it), then the switch would be easy.

sumitsarkar commented 5 years ago

Just a suggestion here would be to not refer to the concrete class when passing it around but rather declare an interface. This way, when we want for example to save a station in a different storage (DB, cloud, microservice, you name it), then the switch would be easy.

Agreed! We can introduce the interface when we have varying implementations for storage.