twitter / finatra

Fast, testable, Scala services built on TwitterServer and Finagle
https://twitter.github.io/finatra/
Apache License 2.0
2.27k stars 405 forks source link

Finatra feature test dependency issues with pants project #546

Closed alilewin closed 4 years ago

alilewin commented 4 years ago

Issues resolving finatra feature test dependencies with a pants project.

Expected behavior

I'm trying to run finatra feature testing with a project that uses pants, and I'm slightly confused on how the third party BUILD file should look and the overall behavior I'm running into. Based on the test dependency docs, I would expect feature testing to work after manually adding all the test-scoped dependencies in my project's third party BUILD:

jar_library(
  name = 'finatra-test',
  jars = [
    scala_jar(org = 'com.twitter', name = 'finatra-http', rev = '20.1.0', classifier='tests'),
  ],
)

jar_library(
  name = 'inject-app-test',
  jars = [
    scala_jar(org = 'com.twitter', name = 'inject-app', rev = '20.1.0', classifier='tests')
  ],
  scope='test'
)

jar_library(
  name = 'inject-core-test',
  jars = [
    scala_jar(org = 'com.twitter', name = 'inject-core', rev = '20.1.0', classifier='tests')
  ],
  scope='test'
)

jar_library(
  name = 'inject-modules-test',
  jars = [
    scala_jar(org = 'com.twitter', name = 'inject-modules', rev = '20.1.0', classifier='tests')
  ],
  scope='test'
)

jar_library(
  name = 'inject-server-test',
  jars = [
    scala_jar(org = 'com.twitter', name = 'inject-server', rev = '20.1.0', classifier='tests')
  ],
  scope='test'
)

Actual behavior

Using the above, I still run into the following error when running my feature tests:

                       [error] missing or invalid dependency detected while loading class file 'HttpServerTrait.class'.
                       [error] Could not access type TwitterServer in package com.twitter.inject.server,
                       [error] because it (or its dependencies) are missing. Check your build definition for
                       [error] missing or conflicting dependencies. (Re-run with `-Ylog-classpath` to see the problematic classpath.)
                       [error] A full rebuild may help if 'HttpServerTrait.class' was compiled against an incompatible version of com.twitter.inject.server.
                       [error] /path/to/HttpServerFile: not found: value injector
                       [error]     val factory = injector.instance[...]

After seeing that, I first tried adding inject-server package in my third party build file, which resolved the above but gave me a similar error with a few other dependencies. I eventually had to also add inject-app and inject-modules for everything to finally run, and once those were resolved I ran into this security issue:

                     Waiting for warmup phases to complete...
                     Waiting for warmup phases to complete...

                     Embedded server MyServer failed to startup: access denied ("java.lang.management.ManagementPermission" "control")
                     java.security.AccessControlException: access denied ("java.lang.management.ManagementPermission" "control")
                        at java.security.AccessControlContext.checkPermission(AccessControlContext.java:472)
                        at java.security.AccessController.checkPermission(AccessController.java:884)
                        at java.lang.SecurityManager.checkPermission(SecurityManager.java:549)
                        at sun.management.Util.checkAccess(Util.java:77)
                        at sun.management.Util.checkControlAccess(Util.java:85)
                        at sun.management.ThreadImpl.setThreadContentionMonitoringEnabled(ThreadImpl.java:189)
                        at com.twitter.jvm.ContentionSnapshot.<init>(ContentionSnapshot.scala:14)
                        at com.twitter.server.handler.ContentionHandler.<init>(ContentionHandler.scala:10)
                        at com.twitter.server.Admin$.adminRoutes(Admin.scala:67)
                        at com.twitter.server.AdminHttpServer$$anonfun$1.apply$mcV$sp(AdminHttpServer.scala:296)
                        at com.twitter.app.App$$anonfun$nonExitingMain$3.apply(App.scala:361)
                        at com.twitter.app.App$$anonfun$nonExitingMain$3.apply(App.scala:361)
                        at scala.collection.mutable.ResizableArray$class.foreach(ResizableArray.scala:59)
                        at scala.collection.mutable.ArrayBuffer.foreach(ArrayBuffer.scala:48)
                        at com.twitter.app.App$class.nonExitingMain(App.scala:361)
                        at /path/to/my/server.MyServer.nonExitingMain(FakeServer.scala:17)
                        at com.twitter.inject.server.EmbeddedTwitterServer$$anonfun$runNonExitingMain$1$$anonfun$apply$mcV$sp$1.apply$mcV$sp(EmbeddedTwitterServer.scala:626)
                        at com.twitter.inject.server.EmbeddedTwitterServer.withLocals(EmbeddedTwitterServer.scala:597)
                        at com.twitter.inject.server.EmbeddedTwitterServer$$anonfun$runNonExitingMain$1.apply$mcV$sp(EmbeddedTwitterServer.scala:624)
                        at com.twitter.inject.server.EmbeddedTwitterServer$$anonfun$runNonExitingMain$1.apply(EmbeddedTwitterServer.scala:624)
                        at com.twitter.inject.server.EmbeddedTwitterServer$$anonfun$runNonExitingMain$1.apply(EmbeddedTwitterServer.scala:624)
                        at com.twitter.util.Try$.apply(Try.scala:26)
                        at com.twitter.util.ExecutorServiceFuturePool$$anon$4.run(FuturePool.scala:140)
                        at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:511)
                        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
                        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
                        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
                        at java.lang.Thread.run(Thread.java:748)

So a few questions - is this even correct? I wouldn't expect testing to require that security and the stack trace leads me to believe it might be using the regular jars, not the test jars. Which makes me think randomly adding the regular jars to the build was not the correct solution. Definitely feels like I'm missing something here, so any help would be appreciated.

Steps to reproduce the behavior

Run regular pants command for finatra feature test: ./pants test /path/to/feature/test/...

cacoco commented 4 years ago

@alilewin sorry to say that we're likely not going to be a lot of help here as we don't support pants in the open source. I don't think we will be able to properly tell you how to specify a pants target which points at a test jar.

As mentioned in the documentation, when publishing the test-jars, the transitive dependencies are not included in the generated pom.xml files thus they need to be added manually. I would have thought that all you needed to do in your project is express the necessary dependencies much as in SBT (which includes other test-jars as well as the main jars -- running the SBT dependency tree was the suggestion for determining these).

alilewin commented 4 years ago

Alright no problem, was worth a try. Are you able to look at that last stack trace and see if the flow looks off to you? For example it goes from the EmbeddedTwitterServer (test jar) -> my server -> com.twitter.App (main jar) -> com.twitter.server, - after reading the docs I assumed it should be triggering the EmbeddedApp somewhere and not just the regular App.

cacoco commented 4 years ago

@alilewin in your last stacktrace it looks like you're using EmbeddedTwitterServer and not EmbeddedApp which are mutually exclusive. That is EmbeddedTwitterServer is not built on EmbeddedApp. Take a look at the documentation here.

What are you trying to test? If it's a TwitterServer, then use EmbeddedTwitterServer, otherwise if it's a direct extension of com.twitter.app.App (or com.twitter.inject.app.App), then use EmbeddedApp.

Documentation for testing is here (apps) and here (servers).

Your exception looks like it's a JVM security manager problem since there is a TwitterServer Admin HTTP Interface handler (ContentionHandler) which attempts to load JVM specific code which your JVM is blocking. You'll need to update the SecurityManager policy to allow access here. But this looks like your Finatra test is fine as it is attempting to start the server, however the Java SecurityManager is preventing that from happening.

Can I ask that you file an issue in the TwitterUtil project with this stacktrace? We should likely be checking the permission in the ContentionSnapshot code before trying to invoke the ManagementFactory to get the ThreadMXBean.

alilewin commented 4 years ago

Thanks for verifying! I'm trying to test a TwitterServer but clearly misread the docs about the EmbeddedApp, so having you confirm that stack trace looks correct is still super helpful.

As for the rest, now that I know the test is setup correctly and there's likely no dependency issue then yes this is just a JVM security manager problem.

As a follow up question, I'd ideally like to avoid having to update the policy to allow this. We don't even use the Admin HTTP Interface, so in general I'm not sure why the feature test is trying to connect when my actual TwitterServer has this line:

  override val disableAdminHttpServer: Boolean = true

Is there anything else I have to do to prevent it from connecting?

As for your last comment, I've filed a TwitterUtil issue here.

cacoco commented 4 years ago

@alilewin correct, setting disableAdminHttpServer should be enough to prevent the TwitterServer Admin HTTP server from starting however, the issue looks like it happens at construction time of the ContentionHandler which this setting does not control.

I'm hopeful we'll address the issue you filed. Thanks!

cacoco commented 4 years ago

@alilewin I'm going to close this issue if thats OK. Hopefully we've addressed the questions here. Please feel free to file a new issue if not. Thanks!