Split Play tests / upgrade non-DB tests to Play 2.3

[richdougherty]

• Split the DB tests out of play-scala and play-scala and into new tests. There are already several separate DB tests so this makes things more consistent. ebean and anorm shouldn't be treated differently from jpa, slick, etc.
• Download sbt so that Play 2.3 can use it.
• Update the non-DB tests to Play 2.3.0. We will need to update the DB tests too, but that's a big job that can be done separately. I'm going to ask the community if they can help with updating the DB tests.

[richdougherty]

Can this be included in Round 10?

There are quite a few other Play tests that will need to be upgraded to Play 2.3, but I'd like to get the minimal play-scala and play-java tests working properly, and then ask the community for assistance with upgrading the rest.

By the way, I don't have a proper test environment so I'm not 100% sure that my script changes are correct. Sorry if I've made some mistakes. I wrote a small test driver to test my setup.py changes so hopefully they will work properly at least. I'm testing on Mac OS so I haven't tested under Windows either.

[bhauer]

Hi @richdougherty. Yes, we'll aim to get this merged in before Round 10. The other guys over here are making their way through the pull requests presently and will likely get here soon. They may have some questions for you when they get started on this one.

[aschneider-techempower]

Can you please update your scripts to reflect changes made by #900 and #915?

[msmith-techempower]

This should be called play-scala-anorm, not play2.

[hamiltont]

@richdougherty since I'm the evil author of 900 and 915, I'll make a pass and note all the areas you need to change. It's going to be a lot of very minor changes mostly intended to allow us to remove the hard-coded dependence on ~/FrameworkBenchmarks.

[hamiltont]

After you rebase onto master, you'll note that all of these now use the IROOT variable, so you'll want to update this to export SBT_HOME=$IROOT/sbt

[hamiltont]

@richdougherty This is an awesome set of changes! I've finished my pass, there were only a few minor things. If you will rebase this onto our master, and update the .travis.yml file to include lines for each new directory you added, then Travis-CI will automatically verify all of your changes and we can merge!

[richdougherty]

@hamiltont, sorry about the delay getting around to this, I've been in the middle of something else. It's rebased now, lets see if it passes validation. It's great to have CI for PRs. :)

I moved the install of sbt into systools/sbt.sh. Hope that's correct.

A question about benchmark_config… Should the outer framework value be the same as the enclosing directory, e.g. play-scala-anorm, but the inner tests/*/framework values be the common name for the framework, e.g. play2?

[hamiltont]

sorry about the delay

It's been two months on our end, so I guess you're forgiven :-)

moved the install of sbt

Looks good to me, thanks for that. It may break a few other tests that have hard-coded the path, but I would prefer to break them so we can find+fix their errors. Your verify will take longer than normal because you modified the toolset/ directory so it has to verify everything (not just the play-* directories), so expect results by tomorrow. Also your result will 100% be a fail because we are verifying everything and that's broken at the moment, so click into details and look at the specific pass/fail results for the play-* tests. If all those pass then you're go for merge

A question about benchmark_config

I frankly don't know...... ping @msmith-techempower ?

[richdougherty]

Cool thanks. I see it was @msmith-techempower who commented on the benchmark_config files.

[richdougherty]

moved the install of sbt

Looks good to me, thanks for that. It may break a few other tests that have hard-coded the path, but I would prefer to break them so we can find+fix their errors

BTW I noticed that the unfiltered test embeds sbt somehow, maybe that's something to refactor at some point.

[msmith-techempower]

Should, but does is not explicitly required, I believe, match the folder-name of this test/framework - this appears to be correct, to my eyes.

[msmith-techempower]

A question about benchmark_config… Should the outer framework value be the same as the enclosing directory

I left some comments in one of your benchmark_config files to explain this -- your solution looks acceptable.

In a perfect world, I think that there would simply be a play2 folder with a single benchmark_config file, and it would have several tests for the different permutations. Let me gather some stuff to create an example gist of what I mean.

[hamiltont]

there would simply be a play2 folder with a single benchmark_config file

If that happened I would love @richdougherty - It provides substantial benefits to other aspects of this project

[msmith-techempower]

Here is that gist -- the non-complete version of all the play2 tests in one mega-benchmark_config. This would also require some folder restructuring, obviously.

[richdougherty]

@msmith-techempower, how does setup.py know which source directory to use for each test? I looked at how FrameworkTest interacts with setup.py and I don't see it providing any config information that would tell setup.py which test it is currently running.

Maybe I need more than one setup file, e.g. setup_java.py, setup_java_ebean.py, etc, so that each setup file knows which test to run?

Of course I could refactor these setup files to be very minimal, and put common code in something like setup_common.py.

[hamiltont]

@richdougherty In this hypothetical, all of the play-* directories would be merged into one play2 directory. The benchmark_config from the gist would then be used, and would call into setup.py for each test.

At the moment setup.py does not have a reliable method to know which test it is running, so a common scheme is to create multiple setup files, calling them something like setup_java_ebean.py and setup_scala.py, etc as needed

[hamiltont]

FrameworkTest interacts with setup.py and I don't see it providing any config information that would tell setup.py which test it is currently running

Yea, unfortunately this is the current state. While you can probably get one setup.py that works for a few of the tests if they all need the same code run, the current better solutoin is to just use multiple setup files. Modify hte benchmark_config to point to the proper setup file for that test and you're good to go

Edit: Oops, I replied before I finished reading your message. You had already figured it out...

[richdougherty]

OK, updated. I'm using a script called generate_config.py to generate the benchmark_config and setup_*.py files. Hopefully I got it right!

[msmith-techempower]

I don't think that this is working because we are forcibly importing the setup_*.py and trying to execute the start and stop functions as if it were a class.

The easiest way to ensure that this isn't the problem is to copypasta the base setup file over and over again and name each the specific setup_java_ebean.py etc. Then change the tiny bits that matter in each.

[richdougherty]

I will just code generate duplicate code for all the setup files if I need to, but I don't think that's the problem. It looks like the start methods are getting called OK, but then there's a problem with relative paths.

[Errno 2] No such file or directory: 'play2-java-ebean/conf/application.conf'

What's the recommended way to get the path to the test directory? I'll try using os.environ['TPATH'].

Note: I'm adding a commit to disable lots of the Travis testing, to make PR validation faster. We obviously don't want to disable the tests, so I'll remove the commit once play2 is working. Unitil then, don't merge this PR! :)

[hamiltont]

you can use $TROOT for bash, or os.environ['TROOT'] for python. I'll try to make a read over your changed setup files in a bit and see if I can help

[hamiltont]

don't merge

roger that. Also, the following few excerpts from the main readme (recently updated) will be helpful:

Use a personal Travis-CI account: This is mainly for your own sanity. Our main Travis-CI can occasionally become clogged with so many pull requests that it takes a day to finish all the builds. If you create a fork and enable Travis-CI.org, you will get your own build queue. This means 1) only your commits/branches are being verified, so there is no delay waiting for an unrelated pull request, and 2) you can cancel unneeded items. This does not affect our own Travis-CI setup at all - any commits added to a pull request will be verifed as normal. Setup is as simple as going to travis-ci.org, using the Log in with Github button, and enabling Travis-CI for your fork. All commit pushes will be automatically verified by Travis-CI, and you will get full log output.

[richdougherty]

OK, I'm getting further now—now it looks like it can't find sbt on the path.

Great idea with using my own Travis account.

[hamiltont]

I think you can find an sbt binary inside $FWROOT/sbt/<somewhere here?>. fwroot is root of this project. FWIW you also have access to $IROOT which is the root of where our installations go. Access these from python with os.environ

[richdougherty]

The sbt problem was caused by fw_get failing for me, even though wget worked locally. If the args --no-check-certificate --trust-server-names are supplied to wget then I get a File name too long error. I needed to use the -O option to control the output filename.

I'm still using absolute paths for all my commands, even though they're on probably the PATH. Using absolute paths is probably overkill but I don't want to fiddle around with paths again now that I've got it working!

Using my personal Travis CI worked well for me, although I did get some errors caused by run-ci.py's _should_run method. I "fixed" the error by stubbing out the method and always returning true.

INFO:run-ci:Using commit range 576dddca9b74...7fc36da74b58
INFO:run-ci:Running `git log --name-only -m --pretty="format:" 576dddca9b74...7fc36da74b58`
fatal: ambiguous argument '576dddca9b74...7fc36da74b58': unknown revision or path not in the working tree.

Once I got everything working in a single directory your new verification code found several problems. I fixed these errors too.

This PR is now ready to merge, if you're happy with it.

[hamiltont]

Using my personal Travis CI worked well for me, although I did get some errors caused by run-ci.py's _should_run method. I "fixed" the error by stubbing out the method and always returning true.

Yea, I've fixed those up in our master. That part is quite tricky and error prone, stubbing it out for testing is totally fine.

I fixed these errors too

Amazing!!

This PR is now ready to merge, if you're happy with it

I've pulled this and run some of the tests myself. You get a tentative LGTM, and if travis agree's I'll merge it tonight. Can't thank you enough for the help

PS - while you're here, let me ask you this...where do I put all the other play-* tests? We now have a play1 and a play2 directory, which makes sense to me. Should all of the other play-* items be merged into those two eventually or are there differences? e.g. play-activate-mysql, play-scala-mongodb, etc. I have no idea if these are play1 or play2 or something entirely different

[richdougherty]

Looks like play2 passed! It'd be great if you can merge this.

I've written up a TODO document of the further work needed for the Play 2 tests. I don't have time to do this myself unfortunately. I was thinking of asking on the Play Framework mailing list to see if anyone wants to step up and maintain the tests. I'll wait until you merge before I send my message.

[hamiltont]

Awesome, this LGTM, I'll merge soon. Thank you for this help!!!

PS - could you hold off on sending that message until tomorrow sometime? I hope to have #1015 merged in by then, which will make the project a lot easier for new people to navigate

[richdougherty]

Great, I'm happy to have that merged. :) I can send that message on Monday if you like.

[hamiltont]

That would actually be great, I'll have a nice buffer of time to finish my
PR as its a big one. Thank you for putting the document together in
addition to all your other help here!

[hamiltont]

@richdougherty Feel free to send the message whenever, my big PR is closed. Also, I realized during it that I completely shot myself in the foot by asking you to merge the play scala and java tests. I ended up refactoring it minorly just because I needed them in separate directories here. Sorry about that!!! On the plus side, now the code for both those tests is great ;-)