Now controls life-cycle of ProjectIndexJobs

[mads-hartmann]

The life-cycle of the ProjectIndexJobs is now managed by IndexJobManager. It listens to open/close/delete events on the projects in the workspace using ProjectChangeObserver.

Tests have been aded for both the ProjectChangeObserver and IndexJobManager

I now manage dependencies between various components using the cake pattern.

Fixes #1001609 #1001610

[scala-jenkins]

Started jenkins job scala-search-3.0.x-2.10-pr-validator at https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-3.0.x-2.10-pr-validator/37/

[scala-jenkins]

Started jenkins job scala-search-master-2.10-pr-validator at https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-master-2.10-pr-validator/38/

[scala-jenkins]

jenkins job scala-search-master-2.10-pr-validator: Failed - https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-master-2.10-pr-validator/38/

[scala-jenkins]

jenkins job scala-search-3.0.x-2.10-pr-validator: Failed - https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-3.0.x-2.10-pr-validator/37/

[mads-hartmann]

Hm, I don't get these errors locally

[mads-hartmann]

Seems like the validator is using the wrong commit. Any ideas @jsuereth @dotta ?

[dotta]

@mads379 Not sure why it happens, but let's give it another try.

[dotta]

PLS REBUILD ALL

[scala-jenkins]

Started jenkins job scala-search-master-2.10-pr-validator at https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-master-2.10-pr-validator/40/

[scala-jenkins]

Started jenkins job scala-search-3.0.x-2.10-pr-validator at https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-3.0.x-2.10-pr-validator/39/

[scala-jenkins]

jenkins job scala-search-master-2.10-pr-validator: Failed - https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-master-2.10-pr-validator/40/

[scala-jenkins]

jenkins job scala-search-3.0.x-2.10-pr-validator: Failed - https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-3.0.x-2.10-pr-validator/39/

[mads-hartmann]

Rebased on master. Lets see what happens.

[scala-jenkins]

Started jenkins job scala-search-master-2.10-pr-validator at https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-master-2.10-pr-validator/41/

[scala-jenkins]

Started jenkins job scala-search-3.0.x-2.10-pr-validator at https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-3.0.x-2.10-pr-validator/40/

[scala-jenkins]

jenkins job scala-search-3.0.x-2.10-pr-validator: Failed - https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-3.0.x-2.10-pr-validator/40/

[scala-jenkins]

jenkins job scala-search-master-2.10-pr-validator: Failed - https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-master-2.10-pr-validator/41/

[mads-hartmann]

@dotta Okay, either the the validator is broken or something is wrong with the PR. checked out scala-ide/master and cherry picked this commit. mvn clean install runs smoothly :)

[dotta]

@mads379 I just checked out your PR, and I can certify the reported failure is real :) I am a bit puzzled on why you can't reproduce it, as it doesn't look like a (bad) timing kind of failure.

[mads-hartmann]

@dotta How did you re-produce it?

[dotta]

@mads379 just a plain mvn clean install. Maybe try mvn clean install -Dtycho.localArtifacts=ignore...

[mads-hartmann]

@dotta Oh, it does indeed fail. I was certain I tried earlier. Oh well, I'll fix it :)

[mads-hartmann]

No, I get another error.

Failed tests:
   corrupIndexExceptionWhenIndexing(org.scala.tools.eclipse.search.jobs.ProjectIndexJobTest):  
   reactsToCloseEvents(org.scala.tools.eclipse.search.ProjectChangeObserverTest): It should've reacted to the close event expected:<true> but was:<false>

[dotta]

Not sure what to say, the failures I get match exactly the ones reported by the kitten.

[mads-hartmann]

Alright, I'll try another computer to see what happens. Thanks for trying it out :)

[dotta]

Did you notice that the IndexJobManagerTest folder inside test-workspace is empty? Did you forget to push some file?

[mads-hartmann]

That was indeed the problem :) I had the folder structure locally but because the folders were empty they were not committed. I've added a dummy file now which seems to fix the problem. The ProjectChangeObserverTest still fails every once in a while. I'll look at it tomorrow.

[scala-jenkins]

Started jenkins job scala-search-3.0.x-2.10-pr-validator at https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-3.0.x-2.10-pr-validator/42/

[scala-jenkins]

Started jenkins job scala-search-master-2.10-pr-validator at https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-master-2.10-pr-validator/43/

[scala-jenkins]

jenkins job scala-search-3.0.x-2.10-pr-validator: Success - https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-3.0.x-2.10-pr-validator/42/

[scala-jenkins]

jenkins job scala-search-master-2.10-pr-validator: Success - https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-master-2.10-pr-validator/43/

[dotta]

Mads, awesome PR! The code is really neat and well tested, great job!

[mads-hartmann]

@dotta While refactoring the code to use ScalaProject everywhere I've encountered a problem. In the ProjectChangeObserver we get passed an IProject when projects are opened or closed. When a project is closed we can't test if it as a Scala project because ScalaPlugin.isScalaProject requires it to be open. So instead I think that ProjectChangeObserver needs to deal with IProjects and we just have to check if a project is a Scala project in IndexJobManager before starting a ProjectIndexJob

[mads-hartmann]

@dotta @dragos I believe that I've found a visibility bug. Not sure if it's in the IDE code of Eclipse. Consider the following example:

@Test
def startsIndexingJobWhenProjectIsCreated() {

  val name = "IndexJobManagerTest-ToBeCreated"
  val latch = new CountDownLatch(1)

  ProjectChangeObserver(onOpen = (p: IProject) => {
    // This runs in another thread. 
    if (p.getName == name) {
      assertTrue(ScalaPlugin.plugin.isScalaProject(p))
      latch.countDown()
    }
  })

  // event
  val p = SDTTestUtils.createProjects(name).head
  assertTrue(ScalaPlugin.plugin.isScalaProject(p.underlying))
  latch.await(EVENT_DELAY, java.util.concurrent.TimeUnit.SECONDS)

  // reaction
  assertTrue(config.isIndexing(project.underlying))
}

The assertion in the onOpen function fails whereas the one on just before latch.await succeeds. Any idea how to fix this?

[dotta]

@mads379 Mmmm, interesting, I'll have a look.

[dotta]

@mads379 Actually, can you add the following additional assertion assertTrue(p.isOpen) right before assertTrue(ScalaPlugin.plugin.isScalaProject(p)) and see if it fails.

[mads-hartmann]

@dotta That one succeeds, so the project is open :)

[mads-hartmann]

@dotta It seems to be the hasNature on IProject that's causing it. It succeeds with hasNature(ScalaPlugin.plugin.natureId) on the test thread but not on the ProjectChangeObserver thread. Perhaps we simply have to force the check on isScalaProject to be executed in the main thread? I'm pretty sure Eclipse provides some functionality for that

[mads-hartmann]

@dotta Yep, the problem is caused by IProject. See https://bugs.eclipse.org/bugs/show_bug.cgi?id=329143 doesn't mention a fix though :/

[mads-hartmann]

I thought that maybe it was just the instance of IProject that was propagated through the events that wasn't updated so I tried

val root = ResourcesPlugin.getWorkspace.getRoot
val p = root.getProject(project.getName)
ScalaPlugin.plugin.isScalaProject(p)

Inside of IndexJobManager but this still returns false when though the assertion is true in the test where the projects are created.

[mads-hartmann]

@dotta I might have found the problem. The way projects are created is that we use

project = ...
project.create(null)
project.open(null)

val description = project.getDescription()
description.setNatureIds(Array(ScalaPlugin.plugin.natureId, JavaCore.NATURE_ID))
project.setDescription(description, null)

setDescription will fire it's own events (according to the docs), so by the time we get the events from the create/open calls the project isn't a in fact a Scala project yet as those events haven't propagated through the system yet. I'll see if I can find an elegant way to handle this.

[mads-hartmann]

Alright, so I just pushed 010bdbc which fixes two things

@dotta pointed out that when he checked out the branch and ran the tests they never terminated. This happened because I did some refactoring and forgot to update a variable :) I updated the tests so they all used EVENT_DELAY to define how long to (max) wait for the latches. That number was previously 10000 because it has been used to define the time in milliseconds before, but with latches we define it in seconds so failing tests would appear to be non-terminating as they were waiting 10000 seconds for the latches ;)

Another issue I found was the test for indexing newly created scala projects failed. This test started failing because I fixed the code such that it only started indexing jobs for Scala projects. Now, the problem was that newly created Scala projects weren't considered Scala projects in the thread running the ProjectChangeObserver, even though ScalaPlugin.plugin.isScalaProject would succeed on the thread that created the project. The reason for this turned out to be quite simple.

The ProjectChangeObserver listened to OPEN events on projects. But they way we create Scala projects in Eclipse require three separate method invocations

project.create(null)
project.open(null)
project.setDescription(description, null) // marks it as a Scala project

So when we reacted to the OPEN event the project wasn't considered a Scala project yet by Eclipse, as the event generated by setDescription hadn't propagated through the system yet.

[scala-jenkins]

Started jenkins job scala-search-3.0.x-2.10-pr-validator at https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-3.0.x-2.10-pr-validator/43/

[scala-jenkins]

Started jenkins job scala-search-master-2.10-pr-validator at https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-master-2.10-pr-validator/44/

[scala-jenkins]

jenkins job scala-search-3.0.x-2.10-pr-validator: Success - https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-3.0.x-2.10-pr-validator/43/

[scala-jenkins]

jenkins job scala-search-master-2.10-pr-validator: Success - https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-master-2.10-pr-validator/44/

[mads-hartmann]

@dotta Pushed a commit that adds life-cycle managing. We use stackable traits to do it, inspired by Daniel Spiewak's talk on the cake pattern (http://nescala.org/, around 26:45). Have a look at Lifecycle.scala, IndexJobManager.scala, SearchPlugin.scala to see how it is used right now

[scala-jenkins]

Started jenkins job scala-search-3.0.x-2.10-pr-validator at https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-3.0.x-2.10-pr-validator/44/

[scala-jenkins]

Started jenkins job scala-search-master-2.10-pr-validator at https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-master-2.10-pr-validator/45/

[scala-jenkins]

jenkins job scala-search-3.0.x-2.10-pr-validator: Success - https://jenkins.scala-ide.org:8496/jenkins/job/scala-search-3.0.x-2.10-pr-validator/44/