Java code under main depends on junit?

[Thomas Weise <th...@apache.org>]

Hi,

Is the following dependency intended or an oversight?

https://github.com/apache/beam/blob/06c70bdf871c5da8a115011b43f8072916cd79e8/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/TestPubsub.java#L32

It appears that dependent code is in test scope.

Should the build flag this (the maven build fails)?

Thanks

Re: Java code under main depends on junit?

[Anton Kedin <ke...@google.com>]

My fault, I'll fix the maven issue.

I added this file and it is not in test intentionally. The purpose of this
class is similar to TestPipeline, in that other packages which depend on
GCP IO can use this class in tests, including integration tests. For
example, right now Beam SQL project depends on GCP IO project and uses both
TestPipeline and TestPubsub in the integration tests. Is there a better
approach for such use case?

Regards,
Anton

On Thu, May 17, 2018 at 4:04 PM Thomas Weise <th...@apache.org> wrote:

> Hi,
>
> Is the following dependency intended or an oversight?
>
>
> https://github.com/apache/beam/blob/06c70bdf871c5da8a115011b43f8072916cd79e8/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/TestPubsub.java#L32
>
> It appears that dependent code is in test scope.
>
> Should the build flag this (the maven build fails)?
>
> Thanks
>
>

Re: Java code under main depends on junit?

[Lukasz Cwik <lc...@google.com>]

I agree with separating it out as a separate sub-project for the same
reason as you specify, just wanted to point out that it was just less bad
with Gradle for internal use as we are doing it right now.

On Fri, May 18, 2018 at 10:35 AM Kenneth Knowles <kl...@google.com> wrote:

> Ah, nice. That means you can actually declare a dependency on test suites
> and get their dependencies in order to run them successfully.
>
> It does mean I can't just argue "it doesn't work" but have to go back to
> arguing that it is a design problem :-)
>
> A "test jar" is a jar containing a bunch of tests you can run, akin to an
> executable. Shouldn't depend on someone's tests in order to use their test
> utilities any more than you would depend on someone's main() program to
> call into a library, and shouldn't design your distributed artifacts to
> encourage this.
>
> Kenn
>
> On Fri, May 18, 2018 at 10:22 AM Lukasz Cwik <lc...@google.com> wrote:
>
>> Note that transitive dependencies in Gradle do work correctly for test
>> configurations so the issue with Maven not handling transitive test
>> dependencies is less important within the project. If we expect end users
>> to use these utilities (in which case they could be using Maven), then as
>> suggested many times before a separate test artifact is needed.
>>
>> On Thu, May 17, 2018 at 5:16 PM Thomas Weise <th...@apache.org> wrote:
>>
>>> Thanks!
>>>
>>> IMO we should at least run "mvn verify -DskipTests" in precommit until
>>> the maven build can be retired (== deleted from master).
>>>
>>>
>>> On Thu, May 17, 2018 at 5:00 PM, Anton Kedin <ke...@google.com> wrote:
>>>
>>>> Opened PR <https://github.com/apache/beam/pull/5404> to fix the
>>>> current build issue, opened BEAM-4358
>>>> <https://issues.apache.org/jira/browse/BEAM-4358> to extract test
>>>> dependencies.
>>>>
>>>> Should we keep maven precommits running for now if we have to fix the
>>>> issues like these? In the PR I had to fix another issue in the same
>>>> project, and I suspect other projects are broken for me for similar reasons.
>>>>
>>>> Regards,
>>>> Anton
>>>>
>>>> On Thu, May 17, 2018 at 4:52 PM Kenneth Knowles <kl...@google.com> wrote:
>>>>
>>>>> I know what you mean. But indeed, test artifacts are unsuitable to
>>>>> depend on since transitive deps don't work correctly. I think it makes
>>>>> sense to have a separate test utility. For the core, one reason we didn't
>>>>> was to have PAssert available in main. But now that we have Gradle we
>>>>> actually can do that because it is not a true cycle but a false cycle
>>>>> introduced by maven.
>>>>>
>>>>> For GCP it is even easier.
>>>>>
>>>>> Kenn
>>>>>
>>>>>
>>>>> On Thu, May 17, 2018, 16:28 Thomas Weise <th...@apache.org> wrote:
>>>>>
>>>>>> It is possible to depend on a test artifact to achieve the same, but
>>>>>> unfortunately not transitively.
>>>>>>
>>>>>> Mixing test utilities into the main artifacts seems undesirable,
>>>>>> since they are only needed for tests. It may give more food to the shading
>>>>>> monster also..
>>>>>>
>>>>>> So it is probably better to create a dedicated test tools artifact
>>>>>> that qualifies as transitive dependency?
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>>
>>>>>> On Thu, May 17, 2018 at 4:17 PM, Kenneth Knowles <kl...@google.com>
>>>>>> wrote:
>>>>>>
>>>>>>> This seems correct. Test jars are for tests. Utilities to be used
>>>>>>> for tests need to be in main jars. (If for no other reason, this is how
>>>>>>> transitive deps work)
>>>>>>>
>>>>>>> We've considered putting these things in a separate package (still
>>>>>>> in main). Just no one has done it.
>>>>>>>
>>>>>>> Kenn
>>>>>>>
>>>>>>> On Thu, May 17, 2018, 16:04 Thomas Weise <th...@apache.org> wrote:
>>>>>>>
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> Is the following dependency intended or an oversight?
>>>>>>>>
>>>>>>>>
>>>>>>>> https://github.com/apache/beam/blob/06c70bdf871c5da8a115011b43f8072916cd79e8/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/TestPubsub.java#L32
>>>>>>>>
>>>>>>>> It appears that dependent code is in test scope.
>>>>>>>>
>>>>>>>> Should the build flag this (the maven build fails)?
>>>>>>>>
>>>>>>>> Thanks
>>>>>>>>
>>>>>>>>
>>>>>>
>>>

Re: Java code under main depends on junit?

[Kenneth Knowles <kl...@google.com>]

Ah, nice. That means you can actually declare a dependency on test suites
and get their dependencies in order to run them successfully.

It does mean I can't just argue "it doesn't work" but have to go back to
arguing that it is a design problem :-)

A "test jar" is a jar containing a bunch of tests you can run, akin to an
executable. Shouldn't depend on someone's tests in order to use their test
utilities any more than you would depend on someone's main() program to
call into a library, and shouldn't design your distributed artifacts to
encourage this.

Kenn

On Fri, May 18, 2018 at 10:22 AM Lukasz Cwik <lc...@google.com> wrote:

> Note that transitive dependencies in Gradle do work correctly for test
> configurations so the issue with Maven not handling transitive test
> dependencies is less important within the project. If we expect end users
> to use these utilities (in which case they could be using Maven), then as
> suggested many times before a separate test artifact is needed.
>
> On Thu, May 17, 2018 at 5:16 PM Thomas Weise <th...@apache.org> wrote:
>
>> Thanks!
>>
>> IMO we should at least run "mvn verify -DskipTests" in precommit until
>> the maven build can be retired (== deleted from master).
>>
>>
>> On Thu, May 17, 2018 at 5:00 PM, Anton Kedin <ke...@google.com> wrote:
>>
>>> Opened PR <https://github.com/apache/beam/pull/5404> to fix the current
>>> build issue, opened BEAM-4358
>>> <https://issues.apache.org/jira/browse/BEAM-4358> to extract test
>>> dependencies.
>>>
>>> Should we keep maven precommits running for now if we have to fix the
>>> issues like these? In the PR I had to fix another issue in the same
>>> project, and I suspect other projects are broken for me for similar reasons.
>>>
>>> Regards,
>>> Anton
>>>
>>> On Thu, May 17, 2018 at 4:52 PM Kenneth Knowles <kl...@google.com> wrote:
>>>
>>>> I know what you mean. But indeed, test artifacts are unsuitable to
>>>> depend on since transitive deps don't work correctly. I think it makes
>>>> sense to have a separate test utility. For the core, one reason we didn't
>>>> was to have PAssert available in main. But now that we have Gradle we
>>>> actually can do that because it is not a true cycle but a false cycle
>>>> introduced by maven.
>>>>
>>>> For GCP it is even easier.
>>>>
>>>> Kenn
>>>>
>>>>
>>>> On Thu, May 17, 2018, 16:28 Thomas Weise <th...@apache.org> wrote:
>>>>
>>>>> It is possible to depend on a test artifact to achieve the same, but
>>>>> unfortunately not transitively.
>>>>>
>>>>> Mixing test utilities into the main artifacts seems undesirable, since
>>>>> they are only needed for tests. It may give more food to the shading
>>>>> monster also..
>>>>>
>>>>> So it is probably better to create a dedicated test tools artifact
>>>>> that qualifies as transitive dependency?
>>>>>
>>>>> Thanks
>>>>>
>>>>>
>>>>> On Thu, May 17, 2018 at 4:17 PM, Kenneth Knowles <kl...@google.com>
>>>>> wrote:
>>>>>
>>>>>> This seems correct. Test jars are for tests. Utilities to be used for
>>>>>> tests need to be in main jars. (If for no other reason, this is how
>>>>>> transitive deps work)
>>>>>>
>>>>>> We've considered putting these things in a separate package (still in
>>>>>> main). Just no one has done it.
>>>>>>
>>>>>> Kenn
>>>>>>
>>>>>> On Thu, May 17, 2018, 16:04 Thomas Weise <th...@apache.org> wrote:
>>>>>>
>>>>>>> Hi,
>>>>>>>
>>>>>>> Is the following dependency intended or an oversight?
>>>>>>>
>>>>>>>
>>>>>>> https://github.com/apache/beam/blob/06c70bdf871c5da8a115011b43f8072916cd79e8/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/TestPubsub.java#L32
>>>>>>>
>>>>>>> It appears that dependent code is in test scope.
>>>>>>>
>>>>>>> Should the build flag this (the maven build fails)?
>>>>>>>
>>>>>>> Thanks
>>>>>>>
>>>>>>>
>>>>>
>>

Re: Java code under main depends on junit?

[Lukasz Cwik <lc...@google.com>]

Note that transitive dependencies in Gradle do work correctly for test
configurations so the issue with Maven not handling transitive test
dependencies is less important within the project. If we expect end users
to use these utilities (in which case they could be using Maven), then as
suggested many times before a separate test artifact is needed.

On Thu, May 17, 2018 at 5:16 PM Thomas Weise <th...@apache.org> wrote:

> Thanks!
>
> IMO we should at least run "mvn verify -DskipTests" in precommit until the
> maven build can be retired (== deleted from master).
>
>
> On Thu, May 17, 2018 at 5:00 PM, Anton Kedin <ke...@google.com> wrote:
>
>> Opened PR <https://github.com/apache/beam/pull/5404> to fix the current
>> build issue, opened BEAM-4358
>> <https://issues.apache.org/jira/browse/BEAM-4358> to extract test
>> dependencies.
>>
>> Should we keep maven precommits running for now if we have to fix the
>> issues like these? In the PR I had to fix another issue in the same
>> project, and I suspect other projects are broken for me for similar reasons.
>>
>> Regards,
>> Anton
>>
>> On Thu, May 17, 2018 at 4:52 PM Kenneth Knowles <kl...@google.com> wrote:
>>
>>> I know what you mean. But indeed, test artifacts are unsuitable to
>>> depend on since transitive deps don't work correctly. I think it makes
>>> sense to have a separate test utility. For the core, one reason we didn't
>>> was to have PAssert available in main. But now that we have Gradle we
>>> actually can do that because it is not a true cycle but a false cycle
>>> introduced by maven.
>>>
>>> For GCP it is even easier.
>>>
>>> Kenn
>>>
>>>
>>> On Thu, May 17, 2018, 16:28 Thomas Weise <th...@apache.org> wrote:
>>>
>>>> It is possible to depend on a test artifact to achieve the same, but
>>>> unfortunately not transitively.
>>>>
>>>> Mixing test utilities into the main artifacts seems undesirable, since
>>>> they are only needed for tests. It may give more food to the shading
>>>> monster also..
>>>>
>>>> So it is probably better to create a dedicated test tools artifact that
>>>> qualifies as transitive dependency?
>>>>
>>>> Thanks
>>>>
>>>>
>>>> On Thu, May 17, 2018 at 4:17 PM, Kenneth Knowles <kl...@google.com>
>>>> wrote:
>>>>
>>>>> This seems correct. Test jars are for tests. Utilities to be used for
>>>>> tests need to be in main jars. (If for no other reason, this is how
>>>>> transitive deps work)
>>>>>
>>>>> We've considered putting these things in a separate package (still in
>>>>> main). Just no one has done it.
>>>>>
>>>>> Kenn
>>>>>
>>>>> On Thu, May 17, 2018, 16:04 Thomas Weise <th...@apache.org> wrote:
>>>>>
>>>>>> Hi,
>>>>>>
>>>>>> Is the following dependency intended or an oversight?
>>>>>>
>>>>>>
>>>>>> https://github.com/apache/beam/blob/06c70bdf871c5da8a115011b43f8072916cd79e8/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/TestPubsub.java#L32
>>>>>>
>>>>>> It appears that dependent code is in test scope.
>>>>>>
>>>>>> Should the build flag this (the maven build fails)?
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>>
>>>>
>

Re: Java code under main depends on junit?

[Thomas Weise <th...@apache.org>]

Thanks!

IMO we should at least run "mvn verify -DskipTests" in precommit until the
maven build can be retired (== deleted from master).


On Thu, May 17, 2018 at 5:00 PM, Anton Kedin <ke...@google.com> wrote:

> Opened PR <https://github.com/apache/beam/pull/5404> to fix the current
> build issue, opened BEAM-4358
> <https://issues.apache.org/jira/browse/BEAM-4358> to extract test
> dependencies.
>
> Should we keep maven precommits running for now if we have to fix the
> issues like these? In the PR I had to fix another issue in the same
> project, and I suspect other projects are broken for me for similar reasons.
>
> Regards,
> Anton
>
> On Thu, May 17, 2018 at 4:52 PM Kenneth Knowles <kl...@google.com> wrote:
>
>> I know what you mean. But indeed, test artifacts are unsuitable to depend
>> on since transitive deps don't work correctly. I think it makes sense to
>> have a separate test utility. For the core, one reason we didn't was to
>> have PAssert available in main. But now that we have Gradle we actually can
>> do that because it is not a true cycle but a false cycle introduced by
>> maven.
>>
>> For GCP it is even easier.
>>
>> Kenn
>>
>>
>> On Thu, May 17, 2018, 16:28 Thomas Weise <th...@apache.org> wrote:
>>
>>> It is possible to depend on a test artifact to achieve the same, but
>>> unfortunately not transitively.
>>>
>>> Mixing test utilities into the main artifacts seems undesirable, since
>>> they are only needed for tests. It may give more food to the shading
>>> monster also..
>>>
>>> So it is probably better to create a dedicated test tools artifact that
>>> qualifies as transitive dependency?
>>>
>>> Thanks
>>>
>>>
>>> On Thu, May 17, 2018 at 4:17 PM, Kenneth Knowles <kl...@google.com> wrote:
>>>
>>>> This seems correct. Test jars are for tests. Utilities to be used for
>>>> tests need to be in main jars. (If for no other reason, this is how
>>>> transitive deps work)
>>>>
>>>> We've considered putting these things in a separate package (still in
>>>> main). Just no one has done it.
>>>>
>>>> Kenn
>>>>
>>>> On Thu, May 17, 2018, 16:04 Thomas Weise <th...@apache.org> wrote:
>>>>
>>>>> Hi,
>>>>>
>>>>> Is the following dependency intended or an oversight?
>>>>>
>>>>> https://github.com/apache/beam/blob/06c70bdf871c5da8a115011b43f807
>>>>> 2916cd79e8/sdks/java/io/google-cloud-platform/src/
>>>>> main/java/org/apache/beam/sdk/io/gcp/pubsub/TestPubsub.java#L32
>>>>>
>>>>> It appears that dependent code is in test scope.
>>>>>
>>>>> Should the build flag this (the maven build fails)?
>>>>>
>>>>> Thanks
>>>>>
>>>>>
>>>

Re: Java code under main depends on junit?

[Anton Kedin <ke...@google.com>]

Opened PR <https://github.com/apache/beam/pull/5404> to fix the current
build issue, opened BEAM-4358
<https://issues.apache.org/jira/browse/BEAM-4358> to extract test
dependencies.

Should we keep maven precommits running for now if we have to fix the
issues like these? In the PR I had to fix another issue in the same
project, and I suspect other projects are broken for me for similar reasons.

Regards,
Anton

On Thu, May 17, 2018 at 4:52 PM Kenneth Knowles <kl...@google.com> wrote:

> I know what you mean. But indeed, test artifacts are unsuitable to depend
> on since transitive deps don't work correctly. I think it makes sense to
> have a separate test utility. For the core, one reason we didn't was to
> have PAssert available in main. But now that we have Gradle we actually can
> do that because it is not a true cycle but a false cycle introduced by
> maven.
>
> For GCP it is even easier.
>
> Kenn
>
>
> On Thu, May 17, 2018, 16:28 Thomas Weise <th...@apache.org> wrote:
>
>> It is possible to depend on a test artifact to achieve the same, but
>> unfortunately not transitively.
>>
>> Mixing test utilities into the main artifacts seems undesirable, since
>> they are only needed for tests. It may give more food to the shading
>> monster also..
>>
>> So it is probably better to create a dedicated test tools artifact that
>> qualifies as transitive dependency?
>>
>> Thanks
>>
>>
>> On Thu, May 17, 2018 at 4:17 PM, Kenneth Knowles <kl...@google.com> wrote:
>>
>>> This seems correct. Test jars are for tests. Utilities to be used for
>>> tests need to be in main jars. (If for no other reason, this is how
>>> transitive deps work)
>>>
>>> We've considered putting these things in a separate package (still in
>>> main). Just no one has done it.
>>>
>>> Kenn
>>>
>>> On Thu, May 17, 2018, 16:04 Thomas Weise <th...@apache.org> wrote:
>>>
>>>> Hi,
>>>>
>>>> Is the following dependency intended or an oversight?
>>>>
>>>>
>>>> https://github.com/apache/beam/blob/06c70bdf871c5da8a115011b43f8072916cd79e8/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/TestPubsub.java#L32
>>>>
>>>> It appears that dependent code is in test scope.
>>>>
>>>> Should the build flag this (the maven build fails)?
>>>>
>>>> Thanks
>>>>
>>>>
>>

Re: Java code under main depends on junit?

[Kenneth Knowles <kl...@google.com>]

I know what you mean. But indeed, test artifacts are unsuitable to depend
on since transitive deps don't work correctly. I think it makes sense to
have a separate test utility. For the core, one reason we didn't was to
have PAssert available in main. But now that we have Gradle we actually can
do that because it is not a true cycle but a false cycle introduced by
maven.

For GCP it is even easier.

Kenn


On Thu, May 17, 2018, 16:28 Thomas Weise <th...@apache.org> wrote:

> It is possible to depend on a test artifact to achieve the same, but
> unfortunately not transitively.
>
> Mixing test utilities into the main artifacts seems undesirable, since
> they are only needed for tests. It may give more food to the shading
> monster also..
>
> So it is probably better to create a dedicated test tools artifact that
> qualifies as transitive dependency?
>
> Thanks
>
>
> On Thu, May 17, 2018 at 4:17 PM, Kenneth Knowles <kl...@google.com> wrote:
>
>> This seems correct. Test jars are for tests. Utilities to be used for
>> tests need to be in main jars. (If for no other reason, this is how
>> transitive deps work)
>>
>> We've considered putting these things in a separate package (still in
>> main). Just no one has done it.
>>
>> Kenn
>>
>> On Thu, May 17, 2018, 16:04 Thomas Weise <th...@apache.org> wrote:
>>
>>> Hi,
>>>
>>> Is the following dependency intended or an oversight?
>>>
>>>
>>> https://github.com/apache/beam/blob/06c70bdf871c5da8a115011b43f8072916cd79e8/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/TestPubsub.java#L32
>>>
>>> It appears that dependent code is in test scope.
>>>
>>> Should the build flag this (the maven build fails)?
>>>
>>> Thanks
>>>
>>>
>

Re: Java code under main depends on junit?

[Thomas Weise <th...@apache.org>]

It is possible to depend on a test artifact to achieve the same, but
unfortunately not transitively.

Mixing test utilities into the main artifacts seems undesirable, since they
are only needed for tests. It may give more food to the shading monster
also..

So it is probably better to create a dedicated test tools artifact that
qualifies as transitive dependency?

Thanks


On Thu, May 17, 2018 at 4:17 PM, Kenneth Knowles <kl...@google.com> wrote:

> This seems correct. Test jars are for tests. Utilities to be used for
> tests need to be in main jars. (If for no other reason, this is how
> transitive deps work)
>
> We've considered putting these things in a separate package (still in
> main). Just no one has done it.
>
> Kenn
>
> On Thu, May 17, 2018, 16:04 Thomas Weise <th...@apache.org> wrote:
>
>> Hi,
>>
>> Is the following dependency intended or an oversight?
>>
>> https://github.com/apache/beam/blob/06c70bdf871c5da8a115011b43f807
>> 2916cd79e8/sdks/java/io/google-cloud-platform/src/
>> main/java/org/apache/beam/sdk/io/gcp/pubsub/TestPubsub.java#L32
>>
>> It appears that dependent code is in test scope.
>>
>> Should the build flag this (the maven build fails)?
>>
>> Thanks
>>
>>

Re: Java code under main depends on junit?

[Kenneth Knowles <kl...@google.com>]

This seems correct. Test jars are for tests. Utilities to be used for tests
need to be in main jars. (If for no other reason, this is how transitive
deps work)

We've considered putting these things in a separate package (still in
main). Just no one has done it.

Kenn

On Thu, May 17, 2018, 16:04 Thomas Weise <th...@apache.org> wrote:

> Hi,
>
> Is the following dependency intended or an oversight?
>
>
> https://github.com/apache/beam/blob/06c70bdf871c5da8a115011b43f8072916cd79e8/sdks/java/io/google-cloud-platform/src/main/java/org/apache/beam/sdk/io/gcp/pubsub/TestPubsub.java#L32
>
> It appears that dependent code is in test scope.
>
> Should the build flag this (the maven build fails)?
>
> Thanks
>
>