Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SPARK-17807][core] Demote scalatest to "provided" in spark-tags. #16303

Closed
wants to merge 1 commit into from

Conversation

vanzin
Copy link
Contributor

@vanzin vanzin commented Dec 16, 2016

This avoids exposing scalatest as a transitive dependencies to projects
that depend on spark-core, but don't declare a direct dependency on
scalatest in test scope.

Tested by checking "mvn dependency:tree" with a small pom file that just
declared a dependency on spark-core; scalatest would show up as "compile"
before the change, now it doesn't show at all.

This avoids exposing scalatest as a transitive dependencies to projects
that depend on spark-core, but don't declare a direct dependency on
scalatest in test scope.

Tested by checking "mvn dependency:tree" with a small pom file that just
declared a dependency on spark-core; scalatest would show up as "compile"
before the change, now it doesn't show at all.
@SparkQA
Copy link

SparkQA commented Dec 16, 2016

Test build #70228 has finished for PR 16303 at commit e5772e2.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@ryan-williams
Copy link
Contributor

ryan-williams commented Dec 16, 2016

I appreciate the quick turn-around on this, though it seems like a mis-use of the provided scope.

FWIW, I am advocating for:

  • mv common/tags/src/{main,test}/java/org/apache/spark/tags;
  • publish the test-jar for the spark-tags module.
    • I know this is trivial in SBT, and I've seen Maven modules do this
    • bdgenomics:utils-misc does it with this config blob afaict?

I am also slightly concerned that while this resolves the issue for Maven, that SBT may process things differently downstream… I'll try to test it on a toy SBT project like the one you described for Maven, though I haven't built+installed a Spark fork in a long time :-\

@srowen
Copy link
Member

srowen commented Dec 16, 2016

This may work in practice, but it doesn't sound like the right change. The scalatest dependency is correctly non-test because it is used by the tags that this module exposes from its non-test code.

One solution is to make spark-tags itself provided, because it won't matter if the annotations are present at runtime, as far as I can tell. It's not really provided, but, doesn't matter.

Another is to move the test-oriented tags to src/test/scala, and have tests depend on its test-jar artifact.

@ryan-williams
Copy link
Contributor

I filed #16311 splitting out a spark-tags test-jar; turns out my second bullet above is a no-op since all Spark modules' test-jars are already automatically published, so that's neat.

As I noted over on #16311, I now feel that we should just move each of the three test-tags to the modules they are pretty explicitly scoped to (in name and usage), and do away with spark-tags' containing these test-tags altogether, but we can discuss that further over there.

@vanzin
Copy link
Contributor Author

vanzin commented Dec 19, 2016

Closing in favor of the other PR.

@vanzin vanzin closed this Dec 19, 2016
ghost pushed a commit to dbtsai/spark that referenced this pull request Dec 22, 2016
Remove spark-tag's compile-scope dependency (and, indirectly, spark-core's compile-scope transitive-dependency) on scalatest by splitting test-oriented tags into spark-tags' test JAR.

Alternative to apache#16303.

Author: Ryan Williams <ryan.blake.williams@gmail.com>

Closes apache#16311 from ryan-williams/tt.
asfgit pushed a commit that referenced this pull request Dec 23, 2016
Remove spark-tag's compile-scope dependency (and, indirectly, spark-core's compile-scope transitive-dependency) on scalatest by splitting test-oriented tags into spark-tags' test JAR.

Alternative to #16303.

Author: Ryan Williams <ryan.blake.williams@gmail.com>

Closes #16311 from ryan-williams/tt.

(cherry picked from commit afd9bc1)
Signed-off-by: Marcelo Vanzin <vanzin@cloudera.com>
asfgit pushed a commit that referenced this pull request Dec 23, 2016
Remove spark-tag's compile-scope dependency (and, indirectly, spark-core's compile-scope transitive-dependency) on scalatest by splitting test-oriented tags into spark-tags' test JAR.

Alternative to #16303.

Author: Ryan Williams <ryan.blake.williams@gmail.com>

Closes #16311 from ryan-williams/tt.

(cherry picked from commit afd9bc1)
Signed-off-by: Marcelo Vanzin <vanzin@cloudera.com>
uzadude pushed a commit to uzadude/spark that referenced this pull request Jan 27, 2017
Remove spark-tag's compile-scope dependency (and, indirectly, spark-core's compile-scope transitive-dependency) on scalatest by splitting test-oriented tags into spark-tags' test JAR.

Alternative to apache#16303.

Author: Ryan Williams <ryan.blake.williams@gmail.com>

Closes apache#16311 from ryan-williams/tt.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants