[jira] [Comment Edited] (CASSANDRA-16704) Fix imports; run tests with packaged dependencies
[
https://issues.apache.org/jira/browse/CASSANDRA-16704?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17361837#comment-17361837
]
Michael Semb Wever edited comment on CASSANDRA-16704 at 6/11/21, 3:13 PM:
--
Testing both patches on this
[branch|https://github.com/apache/cassandra/compare/trunk...thelastpickle:mck/4.0/16704].
Between builds on cassandra-4.0.0 and these patches, the {{lib/}} and
{{build/lib/jars/}} result in identical contents.
But the {{build/test/lib/jars/}} directory contains differences:
{code:java}
❯ diff <(ls build/test/lib/jars/) <(ls ../cassandra/build/test/lib/jars/)
11d10
< assertj-core-3.15.0.jar
15,18d13
< byteman-4.0.6.jar
< byteman-bmunit-4.0.6.jar
< byteman-install-4.0.6.jar
< byteman-submit-4.0.6.jar
22d16
< compile-command-annotations-1.2.0.jar {code}
;that is those jar files no longer appear under {{build/test/lib/jars/}} which
is the intentional of the patch (as they are already "provided" and so under
{{build/lib/jars/}}).
CI:
[!https://ci-cassandra.apache.org/view/patches/job/Cassandra-devbranch/862/badge/icon!|https://ci-cassandra.apache.org/blue/organizations/jenkins/Cassandra-devbranch/detail/Cassandra-devbranch/862/pipeline]
was (Author: michaelsembwever):
Testing both patches.
Between builds on cassandra-4.0.0 and these patches, the {{lib/}} and
{{build/lib/jars/}} result in identical contents.
But the {{build/test/lib/jars/}} directory contains differences:
{code:java}
❯ diff <(ls build/test/lib/jars/) <(ls ../cassandra/build/test/lib/jars/)
11d10
< assertj-core-3.15.0.jar
15,18d13
< byteman-4.0.6.jar
< byteman-bmunit-4.0.6.jar
< byteman-install-4.0.6.jar
< byteman-submit-4.0.6.jar
22d16
< compile-command-annotations-1.2.0.jar {code}
;that is those jar files no longer appear under {{build/test/lib/jars/}} which
is the intentional of the patch (as they are already "provided" and so under
{{build/lib/jars/}}).
CI:
[!https://ci-cassandra.apache.org/view/patches/job/Cassandra-devbranch/862/badge/icon!|https://ci-cassandra.apache.org/blue/organizations/jenkins/Cassandra-devbranch/detail/Cassandra-devbranch/862/pipeline]
> Fix imports; run tests with packaged dependencies
> -
>
> Key: CASSANDRA-16704
> URL: https://issues.apache.org/jira/browse/CASSANDRA-16704
> Project: Cassandra
> Issue Type: Bug
> Components: Build, Test/burn, Test/unit
>Reporter: Angelo Polo
>Assignee: Angelo Polo
>Priority: Normal
> Fix For: 4.0.x, 4.x
>
> Attachments: cleanup-imports.patch, dedup-deps.patch
>
>
> Tests are currently run with a classpath containing _all_ downloaded jars.
> The tests would be more reflective of the behavior of a runtime environment
> if the test classpath only contained jars that are bundled with the binary
> release, together with explicit test dependencies. Ideally we'd use the
> build/lib/ jars for the classpath since that's what gets packaged, but since
> these aren't available at test compile time and should be identical to lib/
> anyway, I've used the later.
> Doing so exposed a couple of references in src/java to
> "org.apache.commons.lang", which is not available at runtime (should be
> "org.apache.commons.lang*3*").
> Attached patch modifies the test classpath, fixes various imports in both
> test/ and src/ classes, and makes some simple substitutions in the tests such
> as using AbstractMap.SimpleEntry in place of
> org.apache.commons.collections.keyvalue.AbstractMapEntry.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-16704) Fix imports; run tests with packaged dependencies
[
https://issues.apache.org/jira/browse/CASSANDRA-16704?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17361837#comment-17361837
]
Michael Semb Wever edited comment on CASSANDRA-16704 at 6/11/21, 3:10 PM:
--
Testing both patches.
Between builds on cassandra-4.0.0 and these patches, the {{lib/}} and
{{build/lib/jars/}} result in identical contents.
But the {{build/test/lib/jars/}} directory contains differences:
{code:java}
❯ diff <(ls build/test/lib/jars/) <(ls ../cassandra/build/test/lib/jars/)
11d10
< assertj-core-3.15.0.jar
15,18d13
< byteman-4.0.6.jar
< byteman-bmunit-4.0.6.jar
< byteman-install-4.0.6.jar
< byteman-submit-4.0.6.jar
22d16
< compile-command-annotations-1.2.0.jar {code}
;that is those jar files no longer appear under {{build/test/lib/jars/}} which
is the intentional of the patch (as they are already "provided" and so under
{{build/lib/jars/}}).
CI:
[!https://ci-cassandra.apache.org/view/patches/job/Cassandra-devbranch/862/badge/icon!|https://ci-cassandra.apache.org/blue/organizations/jenkins/Cassandra-devbranch/detail/Cassandra-devbranch/862/pipeline]
was (Author: michaelsembwever):
Testing both patches.
Between builds on cassandra-4.0.0 and these patches, the {{lib/}} and
{{build/lib/jars/}} result in identical contents.
But the {{build/test/lib/jars/}} directory contains differences:
{code:java}
❯ diff <(ls build/test/lib/jars/) <(ls ../cassandra/build/test/lib/jars/)
11d10
< assertj-core-3.15.0.jar
15,18d13
< byteman-4.0.6.jar
< byteman-bmunit-4.0.6.jar
< byteman-install-4.0.6.jar
< byteman-submit-4.0.6.jar
22d16
< compile-command-annotations-1.2.0.jar {code}
;that is those jar files no longer appear under {{build/test/lib/jars/}} which
is the intentional of the patch (as they are already "provided" and so under
{{build/lib/jars/}}).
CI:
!https://ci-cassandra.apache.org/view/patches/job/Cassandra-devbranch/862/badge/icon|https://ci-cassandra.apache.org/blue/organizations/jenkins/Cassandra-devbranch/detail/Cassandra-devbranch/862/pipeline!
> Fix imports; run tests with packaged dependencies
> -
>
> Key: CASSANDRA-16704
> URL: https://issues.apache.org/jira/browse/CASSANDRA-16704
> Project: Cassandra
> Issue Type: Bug
> Components: Build, Test/burn, Test/unit
>Reporter: Angelo Polo
>Assignee: Angelo Polo
>Priority: Normal
> Fix For: 4.0.x, 4.x
>
> Attachments: cleanup-imports.patch, dedup-deps.patch
>
>
> Tests are currently run with a classpath containing _all_ downloaded jars.
> The tests would be more reflective of the behavior of a runtime environment
> if the test classpath only contained jars that are bundled with the binary
> release, together with explicit test dependencies. Ideally we'd use the
> build/lib/ jars for the classpath since that's what gets packaged, but since
> these aren't available at test compile time and should be identical to lib/
> anyway, I've used the later.
> Doing so exposed a couple of references in src/java to
> "org.apache.commons.lang", which is not available at runtime (should be
> "org.apache.commons.lang*3*").
> Attached patch modifies the test classpath, fixes various imports in both
> test/ and src/ classes, and makes some simple substitutions in the tests such
> as using AbstractMap.SimpleEntry in place of
> org.apache.commons.collections.keyvalue.AbstractMapEntry.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-16704) Fix imports; run tests with packaged dependencies
[
https://issues.apache.org/jira/browse/CASSANDRA-16704?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17361837#comment-17361837
]
Michael Semb Wever edited comment on CASSANDRA-16704 at 6/11/21, 3:09 PM:
--
Testing both patches.
Between builds on cassandra-4.0.0 and these patches, the {{lib/}} and
{{build/lib/jars/}} result in identical contents.
But the {{build/test/lib/jars/}} directory contains differences:
{code:java}
❯ diff <(ls build/test/lib/jars/) <(ls ../cassandra/build/test/lib/jars/)
11d10
< assertj-core-3.15.0.jar
15,18d13
< byteman-4.0.6.jar
< byteman-bmunit-4.0.6.jar
< byteman-install-4.0.6.jar
< byteman-submit-4.0.6.jar
22d16
< compile-command-annotations-1.2.0.jar {code}
;that is those jar files no longer appear under {{build/test/lib/jars/}} which
is the intentional of the patch (as they are already "provided" and so under
{{build/lib/jars/}}).
CI:
!https://ci-cassandra.apache.org/view/patches/job/Cassandra-devbranch/862/badge/icon|https://ci-cassandra.apache.org/blue/organizations/jenkins/Cassandra-devbranch/detail/Cassandra-devbranch/862/pipeline!
was (Author: michaelsembwever):
Testing both patches.
Between builds on cassandra-4.0.0 and these patches, the {{lib/}} and
{{build/lib/jars/}} result in identical contents.
But the {{build/test/lib/jars/}} directory contains differences:
{code:java}
❯ diff <(ls build/test/lib/jars/) <(ls ../cassandra/build/test/lib/jars/)
11d10
< assertj-core-3.15.0.jar
15,18d13
< byteman-4.0.6.jar
< byteman-bmunit-4.0.6.jar
< byteman-install-4.0.6.jar
< byteman-submit-4.0.6.jar
22d16
< compile-command-annotations-1.2.0.jar {code}
;that is those jar files no longer appear under {{build/test/lib/jars/}} which
is the intentional of the patch (as they are already "provided" and so under
{{build/lib/jars/}}).
CI:
!https://ci-cassandra.apache.org/view/patches/job/Cassandra-devbranch/862/badge/icon|id=badgeUrl!
> Fix imports; run tests with packaged dependencies
> -
>
> Key: CASSANDRA-16704
> URL: https://issues.apache.org/jira/browse/CASSANDRA-16704
> Project: Cassandra
> Issue Type: Bug
> Components: Build, Test/burn, Test/unit
>Reporter: Angelo Polo
>Assignee: Angelo Polo
>Priority: Normal
> Fix For: 4.0.x, 4.x
>
> Attachments: cleanup-imports.patch, dedup-deps.patch
>
>
> Tests are currently run with a classpath containing _all_ downloaded jars.
> The tests would be more reflective of the behavior of a runtime environment
> if the test classpath only contained jars that are bundled with the binary
> release, together with explicit test dependencies. Ideally we'd use the
> build/lib/ jars for the classpath since that's what gets packaged, but since
> these aren't available at test compile time and should be identical to lib/
> anyway, I've used the later.
> Doing so exposed a couple of references in src/java to
> "org.apache.commons.lang", which is not available at runtime (should be
> "org.apache.commons.lang*3*").
> Attached patch modifies the test classpath, fixes various imports in both
> test/ and src/ classes, and makes some simple substitutions in the tests such
> as using AbstractMap.SimpleEntry in place of
> org.apache.commons.collections.keyvalue.AbstractMapEntry.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
-
To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-16704) Fix imports; run tests with packaged dependencies
[ https://issues.apache.org/jira/browse/CASSANDRA-16704?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17356571#comment-17356571 ] Angelo Polo edited comment on CASSANDRA-16704 at 6/3/21, 4:43 PM: -- If the 'provided' scope is intentionally part of the test dependencies (and not just the 'test' scope), then does it make sense to remove the exclusion of 'provided' from [test jars resolution|https://github.com/apache/cassandra/blob/3282f5ecf187ecbb56b8d73ab9a9110c010898b0/.build/build-resolver.xml#L178]? Unless there's some other detail with the transitive dependencies, looks like this is what provided is for: "A dependency with this scope is added to the classpath used for compilation and test, but not the runtime classpath." (http://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Dependency_Scope). Then as a bonus, there's no need to duplicate dependencies across provided and test. For example: ./build.xml:733: ./build.xml:734: ./build.xml:735: ./build.xml:736: ./build.xml:830: ./build.xml:831: ./build.xml:832: ./build.xml:833: Though in terms of classpath, we're effectively back where we started since instead of joining the compile and provided scopes from build/lib/ with the test scope in build/test/lib/ we'd be joining the compile scope in lib/ with the test and provided scopes in build/test/lib/. So let me know if you me to remove the modification to build.xml from the patch. At a minimum, I think the two changes to src/ are required for runtime correctness. was (Author: polo-language): If the 'provided' scope is intentionally part of the test dependencies (and not just the 'test' scope), then does it make sense to remove the exclusion of 'provided' from [test jars resolution|https://github.com/apache/cassandra/blob/3282f5ecf187ecbb56b8d73ab9a9110c010898b0/.build/build-resolver.xml#L178]? Unless there's some other detail with the transitive dependencies, looks like this is what provided is for: "A dependency with this scope is added to the classpath used for compilation and test, but not the runtime classpath." ([http://maven.apache.org/guides/introduction/introduction-to-dependency-mechanism.html#Dependency_Scope).] Then as a bonus, there's no need to duplicate dependencies across provided and test. For example: ./build.xml:733: ./build.xml:734: ./build.xml:735: ./build.xml:736: ./build.xml:830: ./build.xml:831: ./build.xml:832: ./build.xml:833: Though in terms of classpath, we're effectively back where we started since instead of joining the compile and provided scopes from build/lib/ with the test scope in build/test/lib/ we'd be joining the compile scope in lib/ with the test and provided scopes in build/test/lib/. So let me know if you me to remove the modification to build.xml from the patch. At a minimum, I think the two changes to src/ are required for runtime correctness. > Fix imports; run tests with packaged dependencies > - > > Key: CASSANDRA-16704 > URL: https://issues.apache.org/jira/browse/CASSANDRA-16704 > Project: Cassandra > Issue Type: Bug > Components: Build, Test/burn, Test/unit >Reporter: Angelo Polo >Assignee: Angelo Polo >Priority: Normal > Fix For: 4.0.x, 4.x > > Attachments: test-with-runtime-deps.patch > > > Tests are currently run with a classpath containing _all_ downloaded jars. > The tests would be more reflective of the behavior of a runtime environment > if the test classpath only contained jars that are bundled with the binary > release, together with explicit test dependencies. Ideally we'd use the > build/lib/ jars for the classpath since that's what gets packaged, but since > these aren't available at test compile time and should be identical to lib/ > anyway, I've used the later. > Doing so exposed a couple of references in src/java to > "org.apache.commons.lang", which is not available at runtime (should be > "org.apache.commons.lang*3*"). > Attached patch modifies the test classpath, fixes various imports in both > test/ and src/ classes, and makes some simple substitutions in the tests such > as using AbstractMap.SimpleEntry in place of > org.apache.commons.collections.keyvalue.AbstractMapEntry. -- This message was sent by Atlassian Jira (v8.3.4#803005) - To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org For additional commands, e-mail: commits-h...@cassandra.apache.org
[jira] [Comment Edited] (CASSANDRA-16704) Fix imports; run tests with packaged dependencies
[
https://issues.apache.org/jira/browse/CASSANDRA-16704?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17356333#comment-17356333
]
Michael Semb Wever edited comment on CASSANDRA-16704 at 6/3/21, 10:54 AM:
--
(To rehash, mostly for my own sake…)
The differences between {{build/lib/jars/}} and {{lib/}} are the following.
Only {{lib/}} contains:
- cassandra-driver-internal-only-3.25.0.zip
- futures-2.1.6-py2.py3-none-any.zip
- geomet-0.1.0.zip
- sigar-bin/
- six-1.12.0-py2.py3-none-any.zip
This is just the additional binary and python dependencies.
Only {{build/lib/jars/}} contains:
- antlr-3.5.2.jar
- assertj-core-3.15.0.jar
- byteman-4.0.6.jar
- byteman-bmunit-4.0.6.jar
- byteman-install-4.0.6.jar
- byteman-submit-4.0.6.jar
- commons-beanutils-1.7.0.jar
- commons-beanutils-core-1.8.0.jar
- commons-collections-3.2.1.jar
- commons-configuration-1.6.jar
- commons-digester-1.8.jar
- commons-el-1.0.jar
- commons-httpclient-3.0.1.jar
- commons-lang-2.4.jar
- commons-math-2.1.jar
- commons-net-1.4.1.jar
- compile-command-annotations-1.2.0.jar
- compress-lzf-0.8.4.jar
- ftplet-api-1.0.0.jar
- ftpserver-core-1.0.0.jar
- ftpserver-deprecated-1.0.0-M2.jar
- hadoop-core-1.0.3.jar
- hadoop-minicluster-1.0.3.jar
- hadoop-test-1.0.3.jar
- hsqldb-1.8.0.10.jar
- jackson-core-asl-1.0.1.jar
- jackson-mapper-asl-1.0.1.jar
- jacocoagent.jar
- jasper-compiler-5.5.12.jar
- jasper-runtime-5.5.12.jar
- jersey-core-1.0.jar
- jersey-server-1.0.jar
- jets3t-0.7.1.jar
- jetty-6.1.26.jar
- jetty-util-6.1.26.jar
- jsp-2.1-6.1.14.jar
- jsp-api-2.1-6.1.14.jar
- jsr305-2.0.2.jar
- jsr311-api-1.0.jar
- kfs-0.3.jar
- mina-core-2.0.0-M5.jar
- netty-bom-4.1.58.Final.pom
- oro-2.0.8.jar
- servlet-api-2.5-6.1.14.jar
- xmlenc-0.52.jar
This should be only the additional "provided" scope.
Maybe there are jar files here that should be removed, i.e. should not even be
part the provided scope?
bq. Tests are currently run with a classpath containing all downloaded jars.
To be accurate {{build/lib/jars/}} is intended to be the [compile+provided
scoped|https://github.com/apache/cassandra/blob/cassandra-4.0-rc1/.build/build-resolver.xml#L174]
dependency tree, while {{lib/}} is only the
[compile|https://github.com/apache/cassandra/blob/cassandra-4.0-rc1/.build/build-resolver.xml#L196]
scoped dependency tree.
bq. The tests would be more reflective of the behavior of a runtime environment
if the test classpath only contained jars that are bundled with the binary
release, together with explicit test dependencies.
Tests can run against those optional dependencies that we don't bundle. I don't
think that is currently the case for any tests, but it can be.
bq. Ideally we'd use the build/lib/ jars for the classpath since that's what
gets packaged, but since these aren't available at test compile time and should
be identical to lib/ anyway, I've used the later.
I'm not quite understanding this statement, if {{build/lib/jars/}} doesn't
exist then neither does {{lib/}}.
The {{lib/}} contents are put together later in the build cycle (under the
{{resolver-dist-lib}} target) than the {{build/lib/jars/}} contents (which are
done under the {{resolver-retrieve-build}} target). There is a valid question
here as to whether we want tests to now depend upon the {{resolver-dist-lib}}
target.
was (Author: michaelsembwever):
(To rehash, mostly for my own sake…)
The differences between {{build/lib/jars/}} and {{lib/}} are the following.
Only {{lib/}} contains:
- cassandra-driver-internal-only-3.25.0.zip
- futures-2.1.6-py2.py3-none-any.zip
- geomet-0.1.0.zip
- sigar-bin/
- six-1.12.0-py2.py3-none-any.zip
This is just the additional binary and python dependencies.
Only {{build/lib/jars/}} contains:
- antlr-3.5.2.jar
- assertj-core-3.15.0.jar
- byteman-4.0.6.jar
- byteman-bmunit-4.0.6.jar
- byteman-install-4.0.6.jar
- byteman-submit-4.0.6.jar
- commons-beanutils-1.7.0.jar
- commons-beanutils-core-1.8.0.jar
- commons-collections-3.2.1.jar
- commons-configuration-1.6.jar
- commons-digester-1.8.jar
- commons-el-1.0.jar
- commons-httpclient-3.0.1.jar
- commons-lang-2.4.jar
- commons-math-2.1.jar
- commons-net-1.4.1.jar
- compile-command-annotations-1.2.0.jar
- compress-lzf-0.8.4.jar
- ftplet-api-1.0.0.jar
- ftpserver-core-1.0.0.jar
- ftpserver-deprecated-1.0.0-M2.jar
- hadoop-core-1.0.3.jar
- hadoop-minicluster-1.0.3.jar
- hadoop-test-1.0.3.jar
- hsqldb-1.8.0.10.jar
- jackson-core-asl-1.0.1.jar
- jackson-mapper-asl-1.0.1.jar
- jacocoagent.jar
- jasper-compiler-5.5.12.jar
- jasper-runtime-5.5.12.jar
- jersey-core-1.0.jar
- jersey-server-1.0.jar
- jets3t-0.7.1.jar
- jetty-6.1.26.jar
- jetty-util-6.1.26.jar
- jsp-2.1-6.1.14.jar
- jsp-api-2.1-6.1.14.jar
- jsr305-2.0.2.jar
- jsr311-api-1.0.jar
- kfs-0.3.jar
- mina-core-2.0.0-M5.jar
- netty-bom-4.1.58.Final.pom
- oro-2.0.8.jar
- servlet-api-2.5-6.1.14.jar
- xmlenc-0.52.jar
This should be the
