twitter / finatra

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

Documentation test dependencies sbt #504

Closed JanMH closed 5 years ago

JanMH commented 5 years ago

Add the actual required dependencies for testing in the documentation.

Hi

I think it would be easier to get started with using finatra if the required test dependencies are directly listed as code in the first steps section of the documentation.

libraryDependencies ++= Seq(
  "com.twitter" %% "finatra-http" % versions.finatra % "test" classifier "tests",
  "com.twitter" %% "finatra-jackson" % versions.finatra % "test" classifier "tests",
  "com.twitter" %% "inject-server" % versions.finatra % "test" classifier "tests",
  "com.twitter" %% "inject-app" % versions.finatra % "test" classifier "tests",
  "com.twitter" %% "inject-core" % versions.finatra % "test" classifier "tests",
  "com.twitter" %% "inject-modules" % versions.finatra % "test" classifier "tests"
)

Thanks

mosesn commented 5 years ago

@JanMH I think we already do this, see here. What do you think is missing there?

JanMH commented 5 years ago

I believe it might be clearer if we write it like this so the necessary information is more compact.


Finatra publishes `test-jars <https://maven.apache.org/guides/mini/guide-attached-tests.html>`__ for most modules. The test-jars include re-usable utilities for use in testing (e.g., the `EmbeddedTwitterServer <https://github.com/twitter/finatra/blob/develop/inject/inject-server/src/test/scala/com/twitter/inject/server/EmbeddedTwitterServer.scala>`__).

To add a test-jar dependency, depend on the appropriate module with the `tests` classifier. Additionally, these dependencies are typically **only needed** in the ``test`` scope for your project. E.g., with `sbt <https://www.scala-sbt.org/>`__:

.. parsed-literal::

    "com.twitter" %% "finatra-http" % "\ |release|\ " % "test" classifier "tests"

or

.. parsed-literal::

    "com.twitter" %% "finatra-thrift" % "\ |release|\ " % "test" classifier "tests"

Since `transitive test-scoped dependencies are not resolved <https://maven.apache.org/plugins/maven-jar-plugin/examples/create-test-jar.html>`__ using this approach, you will need to specify all other required test-scoped dependencies manually:

.. parsed-literal::

    libraryDependencies ++= Seq(
        "com.twitter" %% "finatra-jackson" % "\ |release|\ " % "test" classifier "tests",
        "com.twitter" %% "inject-server" % "\ |release|\ " % "test" classifier "tests",
        "com.twitter" %% "inject-app" % "\ |release|\ " % "test" classifier "tests",
        "com.twitter" %% "inject-core" % "\ |release|\ " % "test" classifier "tests",
        "com.twitter" %% "inject-modules" % "\ |release|\ " % "test" classifier "tests"
    )

For example, the `finatra-http` test-jar depends on the `inject-app` test-jar (among others). You will have to **manually add** a dependency on the `inject-app` test-jar when using the `finatra-http` test-jar since the `inject-app` test-jar will not be resolvedtransitively. 

Using the `sbt-dependency-graph <https://github.com/jrudolph/sbt-dependency-graph>`__ plugin, you can list the dependencies of the `finatra-http` test configuration for the `packageBin` task:

.. code:: bash

    $ ./sbt -Dsbt.log.noformat=true http/test:packageBin::dependencyList 2>&1 | grep 'com\.twitter:finatra\|com\.twitter:inject'
    [info] com.twitter:finatra-http_2.12:...
    [info] com.twitter:finatra-httpclient_2.12:...
    [info] com.twitter:finatra-jackson_2.12:...
    [info] com.twitter:finatra-slf4j_2.12:...
    [info] com.twitter:finatra-utils_2.12:...
    [info] com.twitter:inject-app_2.12:...
    [info] com.twitter:inject-core_2.12:...
    [info] com.twitter:inject-modules_2.12:...
    [info] com.twitter:inject-request-scope_2.12:...
    [info] com.twitter:inject-server_2.12:...
    [info] com.twitter:inject-slf4j_2.12:...
    [info] com.twitter:inject-utils_2.12:...

In this case, when executing the `packageBin` task for `finatra-http` in the test configuration these dependencies are necessary. Unfortunately, this listing does not explicity state if it's the compile-time or the test-jar version of the dependency that is necessary. However, it is safe to assume that if you want a dependency on the `finatra-http` test-jar you will also need to add dependencies on any test-jar from the listed dependencies as well.

Let me know what you think @mosesn Thanks!

ryanoneill commented 5 years ago

Hi @JanMH, we would welcome a pull request if you're interested in submitting one. On its own, we're unlikely to change this anytime soon.