search

betfair / cougar

Cougar is a framework for making building network exposed service interfaces easy.
http://betfair.github.io/cougar
Apache License 2.0
27 stars 18 forks source link

Verify Cougar 3 on Java 8 #69

Closed ManjunathShivakumar closed 9 years ago

ManjunathShivakumar commented 10 years ago

Ensure there is a build track setup with java 8, and runs alongside the primary build.

eswdd commented 10 years ago

Trial run on travis indicates that 158 integration tests fail on JDK8.

https://s3.amazonaws.com/archive.travis-ci.org/jobs/23741796/log.txt

andredasilvapinto commented 9 years ago

Posting the integration tests comment from #2 here, as requested by Simon:

So it took me a bit longer than I expected, but I managed to get the integration tests passing with Java 8.

All the 158 errors reported on #69 were not being caused by any incompatibility with Java 8 but by badly designed assertions. Basically the tests are depending on the order of several elements in the response, which should be irrelevant. In some cases I have changed code in order to fix that behaviour, in other cases I have just changed the order of the elements in the expected object to fit the response.

Here are the problems that I have identified:

expected:<{"id":1,"error":{"message":"DSC-0015","code":-32099},"jsonrpc":"2.0"}> but was:<{"id":1,"jsonrpc":"2.0","error":{"code":-32099,"message":"DSC-0015"}}>

  • Binary Protocol tests do not ensure there are active sessions to the server before running
  • Set order in response order matters

expected:<[aaa, ddd, ccc, bbb]> but was:<[aaa, ccc, bbb, ddd]>

  • Order matters for document assertions

java.lang.AssertionError: Expected: <?xml version="1.0" encoding="UTF-8"?>aaa stringccc stringbbb stringddd string, actual: <?xml version="1.0" encoding="UTF-8"?>aaa stringbbb stringccc stringddd string: Node node value check: Expected :bbb string Actual :ccc string

  • Order of array elements matters for id extraction on com.betfair.testing.utils.cougar.helpers.CougarHelpers#convertBatchedResponseToMap
  • com.betfair.testing.utils.cougar.assertions.AssertionUtils#jettAssertEquals doesn't print the expected vs actual message when comparing with null

I also don't understand the need for manual JSON and XML equality implementations when libraries like Jackson and XMLUnit can give us that for free.

I have pushed a version with which you should be able to get a successful test run with Java 8, https://github.com/andredasilvapinto/cougar/commit/d05c84b73761119f129614c7fa0c3d8f6ab23c67 . Feel free to use it as you find useful.

In any case, it seems that there is no apparent reason to avoid using Java 8 with Cougar 3, or do you know any other issue with it?

andredasilvapinto commented 9 years ago

@eswdd answering the question about Java 7 and Travis. There is no point in testing my changes with Java 7 as the ones that were only element ordering changes will obviously fail.

My intention was just to check if there were incompatibilities with Java 8. Ultimately, if we want to have tests running on both versions we need to refactor them in order to make them not depend on the order of elements.

eswdd commented 9 years ago

@andredasilvapinto precisely what i was going to say. If we're happy running Cougar on Java 8 then we should fix the tests so that they run on both Java 7 & 8 and then continuously run them. Do you know which you just changed the order on? How many are there?

eswdd commented 9 years ago

As far as some of your comments/questions..

Yep, there are a load of badly designed tests, although I suspect the original author didn't understand what they were testing.

The tests are a result of a failed attempt in the past of getting QABA type individuals to write automated tests using a framework which allowed them to author tests in Excel using a framework called JETT. 18 months ago, all the Cougar tests were in that format, as part of open-sourcing, I migrated all of them to TestNG (not by hand I might add) and then cut the framework down to exactly what was needed to run the tests.

So, in answer to the questions about XML and JSON equality operations - they're hangovers from the JETT code, which was written by people learning to program on the job. They've not been replaced because other things have taken priority, but there is an issue for incrementally cleaning up the tests - #73.

andredasilvapinto commented 9 years ago

As you can see by looking at the the diff, out of 158 these were fixed by changing the order of elements:

eswdd commented 9 years ago

All tests now passing on jdk7 and jdk 8.