Added missing tests, new configuration with HikariCP and bumped Play version

[donovanmuller]

Added the remaining tests for play2-java-jpa:

• /fortunes
• /update
• /plaintext

Also added a new benchmark configuration based on play2-java-jpa but using the HikariCP plugin. This allows the default BoneCP connection pool to be replaced with HikariCP.

[richdougherty]

👍 I think this PR looks great. I have a few suggestions but there's no rush on these things. You can easily change them up in your next PR.

[richdougherty]

👍 I think this PR looks great. I have a few suggestions but there's no rush on these things. You can easily change them up in your next PR.

[richdougherty]

I suggest renaming "JPA" to "JPA BoneCP" here so that the difference between the two frameworks is really clear. I understand that BoneCP is the default but I don't think it will hurt to spell out the difference to anyone who is reading the results!

[richdougherty]

Thanks for implementing the plaintext test. Since the test doesn't hit the database, I'd suggest moving the implementation out of the two JPA tests and into the no-DB Java test. How does that sound?

[msmith-techempower]

I would recommend not moving the plaintext tests out of this - this is cleanly organized as-is.

[richdougherty]

I'm suggesting moving the plaintext test out of the play2-java-jpa and play2-java-jpa-hikaridb tests and into play2-java. I think this is nice and clean, but maybe I'm wrong about that?

In general, with the way I've structured the Play 2 tests (and feel free to disagree) is into database tests (play2-java-jpa, play2-java-ebean, play2-scala-anorm, play2-scala-slick) and non-database tests (play2-java, play2-scala).

Each database test uses a different persistence approach, so they are useful for comparing different persistence frameworks. That's why I recommend the database tests only include tests that use the database: db, query, fortune, update.

The non-database tests don't use a persistence framework so I recommend that they are used to test the common part of Play that doesn't rely on a persistence framework. These tests are: json and plaintext.

If we included json and plaintext in each database test then we'd have lots of duplicated tests that are testing the same thing. When people look at the results for json or plaintext they should only see play2-java and play2-scala in the results. It's not helpful in these tests to see comparisons between persistence frameworks.

Here's the test configuration at the moment:

configurations = [
  # Java test for common 
  ('Java',  None,               ['Linux'],            ['json']),
  ('Java',  'Ebean',            ['Linux'],            ['db', 'query']),
  ('Java',  'JPA',              ['Linux'],            ['db', 'query', 'fortune', 'update', 'plaintext']),
  ('Java',  'JPA HikariCP',     ['Linux'],            ['db', 'query', 'fortune', 'update', 'plaintext']),
  ('Scala', None,               ['Linux'],            ['json']),
  ('Scala', 'Anorm',            ['Linux', 'Windows'], ['db', 'query', 'fortune', 'update']),
  ('Scala', 'Slick',            ['Linux'],            ['db', 'query', 'fortune', 'update']),
]

I'm suggesting that ultimately we get it looking like:

configurations = [
  # Java test for common 
  ('Java',  None,               ['Linux'],            ['json', 'plaintext']),
  ('Java',  'Ebean',            ['Linux'],            ['db', 'query', 'fortune', 'update']),
  ('Java',  'JPA',              ['Linux'],            ['db', 'query', 'fortune', 'update']),
  ('Java',  'JPA HikariCP',     ['Linux'],            ['db', 'query', 'fortune', 'update']),
  ('Scala', None,               ['Linux'],            ['json', 'plaintext']),
  ('Scala', 'Anorm',            ['Linux', 'Windows'], ['db', 'query', 'fortune', 'update']),
  ('Scala', 'Slick',            ['Linux'],            ['db', 'query', 'fortune', 'update']),
]

Does that sound OK?

[msmith-techempower]

Okay, with your full explanation, that sounds better to me.

[richdougherty]

Great :)

[donovanmuller]

@richdougherty Have implemented your suggestions here as well as updating Ebean benchmark etc.

[donovanmuller]

@richdougherty Have implemented your suggestions here as well as updating Ebean benchmark etc.