Fix tests

[alexanderjarvis]

Currently get an error when writing integration tests with specs2. I think the trick is to not close the connection to mongodb.

An example stacktrace:

IllegalStateException: this Mongo has been closed (DBTCPConnector.java:207)
[error] com.mongodb.DBTCPConnector._checkClosed(DBTCPConnector.java:123)
[error] com.mongodb.DBTCPConnector.call(DBTCPConnector.java:207)
[error] com.mongodb.DBApiLayer$MyCollection.__find(DBApiLayer.java:313)
[error] com.mongodb.DBApiLayer$MyCollection.__find(DBApiLayer.java:298)
[error] com.mongodb.DBCollection.findOne(DBCollection.java:682)
[error] com.mongodb.DBCollection.findOne(DBCollection.java:661)
[error] com.mongodb.casbah.MongoCollectionBase$class.findOne(MongoCollection.scala:225)
[error] com.mongodb.casbah.MongoCollection.findOne(MongoCollection.scala:897)
[error] com.novus.salat.dao.SalatDAO.findOne(SalatDAO.scala:311)
[error] com.novus.salat.dao.ModelCompanion$class.findOne(ModelCompanion.scala:168)
[error] models.Test$.findOne(Test.scala:20)

[travisbot]

This pull request passes (merged d771a596 into 9265a264).

[wbarksdale]

This is working for me.

[alexanderjarvis]

This is also working for me now, I'm trying to reproduce the same condition, but without any luck so far.

[leon]

I've pushed a 1.1-SNAPSHOT to the repo that i think will fix this.

can you try it out and see if it works.

You will have to add the snapshot repo to your resolvers

resolvers += "OSS Snapshots" at "https://oss.sonatype.org/content/repositories/snapshots/"

[alexanderjarvis]

Thanks Leon,

I wasn't able to break it again but I will try before I update to the snapshot.

For reference, there was someone else with the same problem: http://stackoverflow.com/questions/12170009/still-cant-run-multiple-tests-against-play-fakeapp-with-salat-casbah

[alexanderjarvis]

It is still broken in the 1.1-SNAPSHOT. I think the problem might be that if the connection is closed at the end of the FakeApplication then another test might be trying to use the same connection which is then closed. I think there's been a lot of chatter recently about how the tests don't run sequentially (unless specified).

I'm going to try adding the sequential keyword..

[alexanderjarvis]

Ok so the problem is that I have multiple "should" blocks in my specification and it fails on the second "should".

e.g.

"all" should {

    "send 404 on a bad request" in {
      running(FakeApplication()) {
        routeAndCall(FakeRequest(GET, "/boum")) must beNone        
      }
    }

    "all tests" in {
      running(FakeApplication()) {
        val result = routeAndCall(FakeRequest(GET, "/tests?app_key=abc")).get
        status(result) must equalTo(OK)
        contentType(result) must beSome.which(_ == "application/json")
        contentAsString(result) must equalTo("[]")
      }
    }

    "all without client id" in {
      running(FakeApplication()) {
        val result = controllers.Tests.all(FakeRequest())
        status(result) must equalTo(UNAUTHORIZED)
      }
    }

  }

  "test" should {

    "test with name" in {
      running(FakeApplication()) {
        val result = routeAndCall(FakeRequest(GET, "/tests/button?app_key=abc")).get
        status(result) must equalTo(OK)
        contentType(result) must beSome.which(_ == "application/json")
        contentAsString(result) must equalTo("{\"name\":\"button\"}")
      }
    }

...

[info] TestsSpec
[info] 
[info] all should
[info] + send 404 on a bad request
[info] + all tests
[info] + all without client id
[info]  
[info] test should
[error] ! test with name
[error]     IllegalStateException: this Mongo has been closed (DBTCPConnector.java:207)
[error] com.mongodb.DBTCPConnector._checkClosed(DBTCPConnector.java:123)
[error] com.mongodb.DBTCPConnector.call(DBTCPConnector.java:207)
[error] com.mongodb.DBApiLayer$MyCollection.__find(DBApiLayer.java:313)
[error] com.mongodb.DBApiLayer$MyCollection.__find(DBApiLayer.java:298)
[error] com.mongodb.DBCollection.findOne(DBCollection.java:682)
[error] com.mongodb.DBCollection.findOne(DBCollection.java:661)
[error] com.mongodb.casbah.MongoCollectionBase$class.findOne(MongoCollection.scala:225)
[error] com.mongodb.casbah.MongoCollection.findOne(MongoCollection.scala:897)
[error] com.novus.salat.dao.SalatDAO.findOne(SalatDAO.scala:311)
[error] com.novus.salat.dao.ModelCompanion$class.findOne(ModelCompanion.scala:168)

[leon]

That's weird.

If play-salat is the one closing the connection with reset() it also set's the connection to null which reinstanciates it when called upon again.

https://github.com/leon/play-salat/blob/master/src/main/scala/se/radley/plugin/salat/SalatPlugin.scala#L25

So this shouldn't happen in theory.

[alexanderjarvis]

I think it's a problem with the life-cycle of the Play framework during testing. Just this morning I read something in another thread which said that during tests there are multiple applications running in the same JVM.. so if that is true, then I guess you can't rely on that kind of logic as if something is held statically (like the connection maybe?).

The only way to get around it easily that I found with the play-jongo plugin was to not close the connection when in Test mode, although I don't really understand the specifics and am new to Scala specifications.

[alexanderjarvis]

See: https://github.com/playframework/Play20/pull/386

[alexy]

I can confirm that 1.1-SNAPSHOT fails every test after the first one in ScalaTest as well. I use a fixture which creates a new app every time similarly to what play-salat does in its own test, having a top app and using .copy with configs in the fixture. Apparently the same ModelCompanion object is used?

@alexanderjarvis -- does your patch solve the problem meanwhile?

[alexanderjarvis]

It should do, but I could not figure out how to compile and publish this plugin to my local repo so that I could test it. If you know how, please share!

[alexy]

@alexanderjarvis -- just do sbt publish-local in your play-salat fork, then in your app's sbt console, update and test. Seems to have a different testing behavior in sbt vs IDEA though (looks like working in IDEA but not in sbt).

[alexanderjarvis]

Does update clean all the dependencies then? I'd been using clean and then compile.. Also thanks, I thought that publish-local was just a play command. Glad to hear it works, I'm going to do some work tomorrow so it'll be good to write the tests I should have started with.. I usually write play apps in a TDD manner, so it was starting to bug me that I didn't have any tests.

[alexy]

Alas this was a different branch -- copying the patch over to 1.1-SNAPSHOT seems to have problems in sbt test, but works when scalatests are run from IDEA. So more investigation needed.

[alexanderjarvis]

What other changes are notable in 1.1-SNAPSHOT? I'm happy working with the older branch if it works for testing.

[alexy]

So strangely enough, IDEA runner of ScalaTest tests works fine with my tests, while sbt fails. I wondered whether parallel/sequential has anything to do with it at the sbt level, but then sbt reports test:parallel-execution is false... The first sbt run reports error java.lang.RuntimeException: Server is not started! in at play.api.test.TestServer.stop(Selenium.scala:117) and the second reports errors from scalax.IO.ScalaIOException. We use TestServer for other tests, so I wonder if there's an inter-play here different in sbt and IDEA somehow.

UPDATE: solved by using scalatest 2.0.M4. Now I have consistent repeated tests each wrapped in running(app), with the above patch.

So back to the original patch... @leon -- is it an appropriate solution?

[mattrco]

I can confirm I am having the same issue with 1.0.9 and 1.1-SNAPSHOT. Is there a workaround apart from using the patch above?

If needed, I can replicate this using the sample project and attach the code.

[mattrco]

Further investigation shows that reverting to play-salat 1.0.7 fixes this issue for me.

Of course, I don't yet know what else this may have broken in my project.

[leon]

I've done some testing and back in 1.0.7 we created a new mongo connection every time we fetched a collection.

Which according to the docs isn't the right way to go http://www.mongodb.org/display/DOCS/Java+Tutorial

we should as we are doing now, create a single mongo instance, which in turn then has a connection pool of 10 connections (default) that it alternates against.

and when we are done with the connection we need to call .close() which will close all the underlying threads against mongo.

Part of the problem could be that the ModelCompanion object has a val collection instead of a def collection.

Try changing you model companion object to look like this instead.

object User extends ModelCompanion[User, ObjectId] {
  val dao = new SalatDAO[User, ObjectId](collection = mongoCollection("users")) {}

  def findOneByUsername(username: String): Option[User] = dao.findOne(MongoDBObject("username" -> username))
  def findByCountry(country: String) = dao.find(MongoDBObject("address.country" -> country))
}

This way the modelcompanion will go all the way back to Mongo on every call to the collection instead of hanging on the the val. (I'm not totally shure this is the solution, but it's worth a try)

Give me a shout if this works and I'll release the final version of 1.1

[alexanderjarvis]

This doesn't seem to work unfortunately.

[alexanderjarvis]

slight change to your example (change val to def) and it works!:

def dao = new SalatDAO[User, ObjectId](collection = mongoCollection("users")) {}

[leon]

Could it be that this line https://github.com/novus/salat/blob/master/salat-core/src/main/scala/com/novus/salat/dao/SalatDAO.scala#L40 should read def collection?

because if we look at what SalatDAO is implementing it's a def https://github.com/novus/salat/blob/master/salat-core/src/main/scala/com/novus/salat/dao/DAO.scala#L249

[leon]

In that case I can ask Novus to change it, what do you think?

[leon]

@alexanderjarvis I don´t think it's a good idea to def the whole dao, because that means you are creating a new dao for every call to it.

[alexanderjarvis]

It makes sense for the constructor of SalatDAO to have the same definition as the trait.

Is it expensive to create the SalatDAO?

[alexanderjarvis]

Although in Java, the example for play-jongo gets the collection using a static method, which in turn returns the singleton instance of the plugin which manages the lifecycle of the Mongo object. This works with all the JUnit tests I have for another project.

For play-salat, if the reference to the collection is held in a function using def and we assume that this is therefore held in a static context of a function, or what not, then it should work, just as defining the entire DAO inside def works.

[alexy]

So what's the next step to finalize this? For now I use a patched version and it tests fine.

[leon]

The only thing you wan't to do is add the !Play.isTest when closing connections?

[alexy]

Exactly -- this gets the testing done.

[leon]

I've added the code in the latest snapshot 400b8b3d7d2334251ad1d92cd2ec3829fb902b53

If everything now works as expected, I'll release 1.1

Please comment back when you've tested

[alexy]

Works fine here.

[leon]

1.1 pushed to sonatype, will arrive shortly at central :)