freimair commented 5 years ago

This is a Bisq Network proposal. Please familiarize yourself with the submission and review process.

The whole Bisq project only features 25 test classes, the need for better test coverage is clear. In order to achieve a higher test coverage, we should agree on the used tools/technologies/frameworks for testing before the collection of test frameworks within Bisq gets out of hand. This goal of this proposal is to foster an understanding on which testing frameworks are available and finally, decide on a small set of tools/technologies/frameworks to be used in Bisq.

TL;DR

Based on your inputs below, I compiled the proposal to these 5 lines:

use junit4 for unit testing (because Arquillian does not support junit5)
use TestFX for GUI testing
use mockito for mocking
stay with travis for now
eventually, use Arquillian to do (very) high level integration tests on Bisq

Unit Testing Frameworks

Unit testing frameworks are a convenient way of doing test-driven development when no GUI is involved. As these frameworks are quite mature, integration testing can be done as well. Below, a couple of hand-picked test metrics are discussed. Afterwards, unittest frameworks are evaluated against the metrics. Finally, conclusions are drawn.

Test Metrics

IDE integration Low effort test-driven development/local testing is only possible when a proper IDE integration exists.
Gradle integration provide effortless automated testing
Test classes in Bisq The number of existing test classes in the Bisq code as per 2019-02-15
Parameterized Tests parameterized tests make it easy to get greater test coverage without having to duplicate code
Test Dependencies Given Test2 depends on Test1 and Test1 fails, Test2 is skipped. Useful in automated testing as it keeps the testing load down in case of a failure. Useful in test design, as basic functionality does not have to be considered for special tests which results in less editing of test code to match altered features.

Comparison

	IDE integration	Gradle integration	Test classes in Bisq	Parameterized Tests	Test Dependencies
JUnit4	+	+	18	o	-
JUnit5 "jupiter"	+	+	7 (only in monitor)	+	-
TestNG	+	+	0	+	+
Spock	?	?	0	?	?

Conclusion

JUnit4 is outdated. Spock, being a functional testing framework, seems the most powerful, yet, it is quite different to use than the classic testing frameworks. TestNG allows for resource saving automated tests, is, however, not supported by at least one GUI testing framework.

GUI Testing

Automated GUI testing is more delicate then simple unit testing. However, given Bisq uses JavaFX only, there are a few testing frameworks which integrate well with unit testing frameworks. Below, a couple of hand-picked test metrics are discussed. Afterwards, available frameworks are evaluated against the metrics. Please be aware, that only frameworks compatible with JavaFX are listed. Finally, conclusions are drawn.

Metrics

Supported UnitTest Frameworks Which unittest frameworks the GUI testing framework in question integrates with.

Comparison

	IDE integration	Gradle integration	Test classes in Bisq	Supported UnitTest frameworks	easy to use
TestFX	+	+	0	JUnit4, JUnit5, Spock	+
Jemmy	o	o	0	-	o
Automaton	?	?	?	?	?

Conclusion

It looks like TestFX is the only option as of now. Jemmy does rely on a void main(.) for running the tests, Automaton seems pretty outdated (last release 2 year ago) and dead (last change 1 year ago).

Mocking Frameworks

Test Metrics

Migration the efforts of migrating to the respective framework
IDE integration it does not clutter up the testing efforts
usage `rep -R -l --include *.java | wc -l

Comparison

	Migration	IDE integration	usage
jmock	?	?	0
mockito	+/-¹	?	32
jmockit	?	-	1
[powermock]()	?	?	23

¹ we have someone with experience, that is a +; however, as Bisq (today) is very near to dependency hell, there will be some work involved clearing Bisq to a point where we can use mockito everywhere, so that is -

thanks @christophsturm for your input:

reasons for mockito:

I know mockito best from previous projects, so I know pretty well how to convert all tests to mockito, with jmockit I would have to read up on it.

Mockito seems to be the most used mocking lib in java land

Our jmockit setup is really complicated, we use 2 different jmockit versions, one in p2p and a different one in the other modules.

Jmockit uses an javaagent which not always works so well in the IDE

last reason and for me the main reason: jmockit and powermock both have powerful static mocking methods which mockito does not have by design, and I think simpler mocking enforces better design.

Conclusion

Mockito seems to be the way to go.

Continuous Integration

Continuous integration is pretty much standard nowadays. However, Bisq only uses the compile-part of CI.

Test Metrics

Git support whether the product can be neatly integrated into Github
build on Win/OSX/Linux whether the product can build on different OSs
free well, ahm, free

Comparison

	Git support	build on Win/OSX/Linux	free
Travis	x	x	x
Jenkins	x	x	-¹

¹ Jenkins is OpenSource and free, however, someone has to host Jenkins and the required OSs

Conclusion

Stay with Travis for as long as possible. Iff we run out of "Travis", we can easily migrate to a (probably self-hosted) Jenkins installation.

A take on integration testing

I just had a call with @blabno and he showed me what Arquillian can do. Key features I picked up from that demo:

fire up multiple docker containers
control them from unit tests (see the example here)
travis is capable of running this test (click)

what it can not do is

junit5 and testng

Overall Conclusion

It is time to conclude this discussion and set it in action. See TL;DR above for the final proposal.

Aftermath

Having decided on the testing frameworks, contributors can freely create tests. In the long run, efforts required for manual testing should be reduced while increasing test coverage and therefore, the quality of Bisq.

Call for contribution

I have started a sketch on how testing frameworks can be compared. I also filled in unit test frameworks as I have some experience with them. I, however, do not have any experience with GUI testing frameworks. If you can contribute something to this proposal, please do so! Thanks!

ManfredKarrer commented 5 years ago

25 test classes

I am pretty sure its much more (I think its 166). How did you get to that number? But agree we have way too low test coverage.

ManfredKarrer commented 5 years ago

Could you add mocking frameworks?

@blabno @ripcurlx @oscarguindzberg Can you add your comments and which test and mock frameworks you use and why you prefer it?

oscarguindzberg commented 5 years ago

This is not my main area of expertise. I use mostly junit. I remember I used jmock and mockito in the past.

devinbileck commented 5 years ago

Thanks @freimair for putting this together! TestFX has been on my todo list for some time, just haven't had the time to look into it yet.

blabno commented 5 years ago

TestNg is a bit (tiny bit) more powerful than junit, but is less known, so I would stick with junit to keep barrier for new Devs low.

I do not like parameterized tests. The name of the test should tell what scenario it tests. Unit tests must be dead simple, this short, of they're not, they will not be maintainable and will lose documentation valor. When parameterized test fails you don't know what the scenario was by looking at test report. You have to inspect the code, which is a pain when tests fail on ci.

As to mocking frameworks, I've used mockito a lot and it has everything needed. It does not support mocking of static methods, but if you need to mock such methods than your design is wrong IMHO.

We should also consider Arquillian as an integration testing framework. It comes with a lot of plugins, but the most useful for us is the Cube, which allows to spin up docker containers and run tests against them. I'm using this heavily in the API development and it allows me to run the whole trade flow on regtest with Alice, Bob and arbitrator bisque nodes in separate containers, as well as seednode and bitcoind node. I bet we could write tests that would work on testnet or even mainnet (that would of course cost is money in terms of transaction and trade fees). IMHO Arquillian is future for Bisq testing effort, once we get the API running.

Right now for me the biggest problem with unit testing Bisq is the amount of dependencies that need to be mocked. The given part takes 20-30 lines, which makes the whole test unreadable, this defeating it's purpose.

Summing up: I'm for junit4 and Arquillian. Unfortunately Arquillian does not support junit5.

freimair commented 5 years ago

It looks like Arquillian does support JUnit 5 and TestNG: http://arquillian.org/docs/#test-runners

ad parameterized tests: hm. TestNG does provide the input data (parameter) for the failed test. Aside from that, it is IMHO possible to do meaningful testing and it saves on boilerplate code. For example (JUnit5):

@ParameterizedTest
@ValueSource(strings = { "empty", "no interval", "typo" })
public void basicConfigurationError(String configuration) {
    Dummy DUT = new Dummy();
    DUT.configure(configurationLut.get(configuration));
    Assert.fail("Config got accepted");
}

ad biggest problem: aggreed. a lot of refactoring is due...

blabno commented 5 years ago

@freimair they have an error on their website. Arquillian does not support Junit5 as it is not compatible with Junit4. There are still open issues about Junit5 integration. I've tested it locally.

christophsturm commented 5 years ago

I think for new code we should use junit5 and mockito. For ui code probably testfx, but that will need help from @ripcurlx as I have never worked with javafx and I have no idea whats a good way to test it. As always with ui testing the best way is probably to get as much code as possible out of the view classes.

blabno commented 5 years ago

What are the killer features of junit5 that you would miss with junit4?

christophsturm commented 5 years ago

there is really not much difference. in junit5 you can give tests better names. I'm only suggesting junit5 because we already use junit5 in one module. staying with junit4 for now is also fine.

blabno commented 5 years ago

I have no experience in testing JavaFX UI, but apart from that I agree with @freimair's conclusions (junit4, mockito, arquillian, travis).

christophsturm commented 5 years ago

was that the conclusion? I think it should be junit5 and mockito. and ci is pretty interchangable, but I think circleCI > travisCI. I'm also not sure what arquillian gives us, integration tests should be light weight and are really easy to do without an external lib.

freimair commented 5 years ago

Arquillian still does not work with junit5...

christophsturm commented 5 years ago

thats exactly my point. every framework that we depend on makes us less flexible.

blabno commented 5 years ago

Arquillian allows to spin up docker containers with whatever you want, and we need at least seednode and bitcoind. In case of API which is just around the corner I put the whole bisq client into the container and thus have truly isolated Alice, Bob and Arbitrator.

christophsturm commented 5 years ago

If arquillian needs so heavy test framework integration, and after 2 years of junit5 being the standard testing framework still does not support it, it's probably not the right way.

I have always just spun up my own docker containers for integration tests, its really easy and fast. I have some kotlin code laying around for that.

blabno commented 5 years ago

In the post above you have stated that there is not much difference between junit 4 and 5. So why pushing so hard for that and locking out from using very useful integration testing tool?

christophsturm commented 5 years ago

here's an example for how i run selenium integration tests via docker selenium in a private project of mine:

    fun chartTest() {
        assumeTrue(DockerUtils.dockerWorks())
        val image = "selenium/standalone-chrome:3.14.0"
        val port = "4444"
        val container = DockerUtils.startContainer(image, port)
        val serverPort = TestUtils.freePort()
        val server = KtorServer(serverPort).apply { start() }
        try {
            val chrome = TestUtils.await {
                RemoteWebDriver(
                    URL("http://${DockerUtils.hostName()}:${container.port}/wd/hub"), DesiredCapabilities.chrome()
                )
            }
            chrome.get("https://127.0.0.1/index.html")
        } finally {
            server.stop()
            container.rmf()
        }

    }

christophsturm commented 5 years ago

IMO integration testing is just too important to lock into the design philosophy of a project like arquillian. IT needs creativity and flexibility and the less code and lightweight approach has always worked better for me.

take it from someone who always strives for 95+ percent test coverage, testing is hard and testing only the easy parts is useless. and arquillian maybe helps with the easy parts but makes the hard parts impossible

christophsturm commented 5 years ago

@blabno if you want i can take a look at your arquillian usage in your pr and try to sketch out how i would do it without arquillian and what the upsides would be.

blabno commented 5 years ago

I also strive for high test coverage and I can assure you that Arquillian is the way to go. I have been using it extensively with Bisq API over last year and a half and it made my life a lot easier. It truly isolates Bisq clients which is very important for e2e tests. And it does not stop you from writing lower level integration tests without Arquillian. You apply Arquillian only to the test classes you choose.

So I'm not speaking from the position of somebody who just saw a cool toy and wants to play with it. I've been using it heavily on this project and it allowed me to write very reliable e2e tests that caught a lot of inconsistencies when the core was changing.

Please go ahead and try to sketch an alternative but try to do it against this code: https://github.com/blabno/exchange/blob/api-offer/api/src/testIntegration/java/bisq/api/http/OfferEndpointIT.java

christophsturm commented 5 years ago

yeah that's a very impressive test. But I think features like InSequence are antipatterns, and also probably the reason why arquillian needs to deeply tie into the testing framework. Testing frameworks want to run tests in random order, or possibly in parallel. So if something needs an order i would just do one test method and call the steps from there.

christophsturm commented 5 years ago

Also I was not aware that we are already using arquillian so much. I'm not saying that we need to rewrite all of this now. I still think that we can write tests with the same readability without arquillian, just by using a small docker utility class, and doing things that need to be in sequence in one umbrella test method. Are there other features of arquillian that you like?

blabno commented 5 years ago

Arquillian is used by API, which is not part of official code, but I hope it will soon be (being reviewed right now). The main features of Arquillian I use is start/stop multiple docker containers per test or per class, and orchestrate those containers, so that they can talk to each other (just like that OfferEndpointIT.java shows). Arquillian also automatically extracts logs from the containers and outputs them after test class, which is very useful for troubleshooting CI failures.

All that seems trivial and initially I started writing my own docker wrapper for that but it turned to be harder than I thought.

christophsturm commented 5 years ago

where in the offerEndPointIT does it configure the orchestration of the containers? i see it just mapping docker ports to local ports. also are the docker features useable without junit integration?

ghubstan commented 5 years ago

Here is some info for filling in some (spock) blanks in the test framework comparison table.

IDE Integration: Experimental stage

Gradle Integration: +

Parameterized Tests: + Groovy example:

def "sample parameterized test"() {   
given:  
        def app = new CalculatorApp()    
when:  
        def resultSum = app.add(input1, input2)     
then:  
    resultSum == expectedResult      
where:  
    input1 |input2  |expectedResult  
    10     |15      |25  
    -4     |6       |2  
}

Test Dependencies: Groovy, JUnit, Jetbrains Annotations, ant 1.9.7, ow2.asm 7.0, net.bytebuddy 1.9.3, cglib 3.2.9, objenesis 2.6

christophsturm commented 5 years ago

IMO groovy is becoming more or less obsolete and if we are not tied to java we should just use kotlin for unit tests. https://strikt.io is a really great assertion lib, and minutest(https://github.com/dmcg/minutest) is the nicest test runner IMO, and it works with junit4 and junit 5. also see http://oneeyedmen.com/my-new-test-model.html

freimair commented 5 years ago

agreed on in cycle 4.

bisq-network / proposals

Finding a unified test framework for Bisq #74

TL;DR

Unit Testing Frameworks

Test Metrics

Comparison

Conclusion

GUI Testing

Metrics

Comparison

Conclusion

Mocking Frameworks

Test Metrics

Comparison

Conclusion

Continuous Integration

Test Metrics

Comparison

Conclusion

A take on integration testing

Overall Conclusion

Aftermath

Call for contribution