OAK-10367: update org.apache.sling.testing.osgi-mock dependency by mbaedke · Pull Request #1044 · apache/jackrabbit-oak

mbaedke · 2023-08-01T10:35:16Z

No description provided.

kwin · 2023-08-01T10:48:55Z

oak-auth-external/pom.xml

            <artifactId>org.osgi.service.component</artifactId>
            <scope>provided</scope>
        </dependency>
+        <dependency>


which code changes requires this new compile classpath change?

No code changes, these were listed as used undeclared dependencies by the dependency plugin.

kwin · 2023-08-01T10:49:47Z

oak-auth-external/pom.xml

+            <artifactId>org.apache.sling.testing.osgi-mock.junit4</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>


junit4 depends on core, therefore you only reference the former in the dependency usually in this case (https://sling.apache.org/documentation/development/osgi-mock.html#maven-dependency)

Again a used undeclared dependency, which I usually declare explicitly as generally recommended. In this particular case, it's admittedly unnecessary.

kwin · 2023-08-01T10:51:33Z

oak-auth-external/pom.xml

+            <artifactId>org.apache.sling.testing.osgi-mock.core</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>


Why all the additional dependencies? Isn't the transitive dependencies from osgi-mock enough?
It is unusual to reference OSGi Spec chapter dependencies only in test classpath. In tests those are only used usually if the to-be tested code relies on those spec chapters as well.

The integration tests will fail without them.

but all those are transitive ones of https://github.com/apache/sling-org-apache-sling-testing-osgi-mock/blob/45fb63cb61615663e10ed3c9e612a3a3f6161c3e/core/pom.xml#L42 and the ITs don't leverage those APIs/spec chapter directly (only the Sling Testing Mock uses them).

Yeah. But the tests will fail anyway. Do you have a suggestion?
Maven dependency analysis does not see reflective use.

kwin · 2023-08-01T13:41:37Z

I am not convinced, that we should make all those dependencies direct ones. Please only make those direct dependencies which are really used in Oak code and also please do that dependency cleanup in a separate commit to ease to distinguish it from the actual update of the testing dependency.

mbaedke · 2023-08-02T06:59:27Z

I am not convinced, that we should make all those dependencies direct ones. Please only make those direct dependencies which are really used in Oak code

If the dependency plugin lists them as used and undeclared, they are (in most cases) used in the project code, no?
In particular, org.osgi.service.component.annotations is used in oak-auth-external.
I don't have strong feelings about this, I just wonder.

and also please do that dependency cleanup in a separate commit to ease to distinguish it from the actual update of the testing dependency.

Agreed.

mbaedke · 2023-08-02T07:12:52Z

...-tar/src/test/java/org/apache/jackrabbit/oak/segment/SegmentBlobTrackerRegistrationTest.java

        properties.put(SegmentNodeStoreService.REPOSITORY_HOME_DIRECTORY, repoHome);
-        service = context.registerInjectActivateService(new SegmentNodeStoreService(), properties);
+        SegmentNodeStoreService service = new SegmentNodeStoreService();
+


@kwin Would you take a look at the following lines? I wonder if MockBundleContext introduced a bug in the new version or if I'm missing something.

Reverted changes that are not related to o.a.sling.testing.osgi-mock. Removed redundant explicit declaration of transitive dependencies of o.a.sling.testing.osgi-mock.junit4.

reschke · 2023-08-02T15:08:01Z

I'm getting test failures:

[ERROR] Tests run: 6, Failures: 0, Errors: 6, Skipped: 0, Time elapsed: 0.51 s <<< FAILURE! - in org.apache.jackrabbit.oak.segment.azure.Azu
reSegmentStoreServiceTest
[ERROR] deactivate(org.apache.jackrabbit.oak.segment.azure.AzureSegmentStoreServiceTest)  Time elapsed: 0.009 s  <<< ERROR!
java.lang.NoClassDefFoundError: org/osgi/service/event/EventAdmin
        at java.base/java.lang.ClassLoader.defineClass1(Native Method)
        at java.base/java.lang.ClassLoader.defineClass(ClassLoader.java:1016)
        at java.base/java.security.SecureClassLoader.defineClass(SecureClassLoader.java:174)
        at java.base/jdk.internal.loader.BuiltinClassLoader.defineClass(BuiltinClassLoader.java:800)
        at java.base/jdk.internal.loader.BuiltinClassLoader.findClassOnClassPathOrNull(BuiltinClassLoader.java:698)
        at java.base/jdk.internal.loader.BuiltinClassLoader.loadClassOrNull(BuiltinClassLoader.java:621)
        at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:579)
        at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:178)
        at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:521)
        at org.apache.sling.testing.mock.osgi.context.OsgiContextImpl.registerDefaultServices(OsgiContextImpl.java:68)
        at org.apache.sling.testing.mock.osgi.context.OsgiContextImpl.setUp(OsgiContextImpl.java:49)
        at org.apache.sling.testing.mock.osgi.junit.OsgiContext.access$100(OsgiContext.java:35)
        at org.apache.sling.testing.mock.osgi.junit.OsgiContext$1.before(OsgiContext.java:79)
        at org.junit.rules.ExternalResource$1.evaluate(ExternalResource.java:50)
        at org.junit.runners.ParentRunner$3.evaluate(ParentRunner.java:306)
        at org.junit.runners.BlockJUnit4ClassRunner$1.evaluate(BlockJUnit4ClassRunner.java:100)
        at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:366)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:103)
        at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:63)
        at org.junit.runners.ParentRunner$4.run(ParentRunner.java:331)
        at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:79)
        at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:329)
        at org.junit.runners.ParentRunner.access$100(ParentRunner.java:66)
        at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:293)
        at com.arakelian.docker.junit.DockerRule$StatementWithDockerRule.evaluate(DockerRule.java:76)
        at org.apache.jackrabbit.oak.blob.cloud.azure.blobstorage.AzuriteDockerRule$1.evaluate(AzuriteDockerRule.java:112)
        at org.junit.rules.RunRules.evaluate(RunRules.java:20)

reschke

see #1044

mbaedke · 2023-08-03T13:33:55Z

Yes, my Docker isn't working properly, which leads to test cases silently being skipped (obviously an issue of it's own). I'm working on it.

Added missing test dependencies.

mbaedke · 2023-08-04T08:21:08Z

Integration test now succeed for me.

* OAK-10367: update org.apache.sling.testing.osgi-mock dependency * OAK-10367: update org.apache.sling.testing.osgi-mock dependency Reverted changes that are not related to o.a.sling.testing.osgi-mock. Removed redundant explicit declaration of transitive dependencies of o.a.sling.testing.osgi-mock.junit4. * OAK-10367: update org.apache.sling.testing.osgi-mock dependency Added missing test dependencies. --------- Co-authored-by: Manfred Baedke <gre55877@adobe.com>

OAK-10367: update org.apache.sling.testing.osgi-mock dependency

71502cc

mbaedke requested review from kwin and reschke August 1, 2023 10:40

kwin reviewed Aug 1, 2023

View reviewed changes

mbaedke commented Aug 2, 2023

View reviewed changes

OAK-10367: update org.apache.sling.testing.osgi-mock dependency

9d8fc1c

Reverted changes that are not related to o.a.sling.testing.osgi-mock. Removed redundant explicit declaration of transitive dependencies of o.a.sling.testing.osgi-mock.junit4.

reschke requested changes Aug 2, 2023

View reviewed changes

OAK-10367: update org.apache.sling.testing.osgi-mock dependency

f7c2761

Added missing test dependencies.

mbaedke requested a review from reschke August 4, 2023 08:21

reschke approved these changes Aug 8, 2023

View reviewed changes

reschke merged commit d6c402c into apache:trunk Aug 9, 2023

Conversation

mbaedke commented Aug 1, 2023

Uh oh!

kwin Aug 1, 2023

Choose a reason for hiding this comment

Uh oh!

mbaedke Aug 1, 2023

Choose a reason for hiding this comment

Uh oh!

kwin Aug 1, 2023

Choose a reason for hiding this comment

Uh oh!

mbaedke Aug 1, 2023

Choose a reason for hiding this comment

Uh oh!

kwin Aug 1, 2023

Choose a reason for hiding this comment

Uh oh!

mbaedke Aug 1, 2023

Choose a reason for hiding this comment

Uh oh!

kwin Aug 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mbaedke Aug 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kwin commented Aug 1, 2023

Uh oh!

mbaedke commented Aug 2, 2023

Uh oh!

mbaedke Aug 2, 2023

Choose a reason for hiding this comment

Uh oh!

reschke commented Aug 2, 2023

Uh oh!

reschke left a comment

Choose a reason for hiding this comment

Uh oh!

mbaedke commented Aug 3, 2023

Uh oh!

mbaedke commented Aug 4, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

kwin Aug 1, 2023 •

edited

Loading

mbaedke Aug 2, 2023 •

edited

Loading