[junit5] Make BundleContextExtension hierarchical

Fixes #113, fixes #114

Also improved test fidelity by using EngineTestKit

Fixes #116.

Signed-off-by: Fr Jeremy Krieg <fr.jkrieg@greekwelfaresa.org.au>

org.osgi.test.common/pom.xml

@@ -57,6 +57,10 @@
 			<groupId>org.assertj</groupId>
 			<artifactId>assertj-core</artifactId>
 		</dependency>
+		<dependency>
+			<groupId>org.mockito</groupId>
+			<artifactId>mockito-core</artifactId>
+		</dependency>
 	</dependencies>
 
 	<build>

org.osgi.test.common/src/main/java/org/osgi/test/common/context/CloseableBundleContext.java

@@ -21,6 +21,7 @@
 
 import java.io.InputStream;
 import java.lang.reflect.InvocationHandler;
+import java.lang.reflect.InvocationTargetException;
 import java.lang.reflect.Method;
 import java.lang.reflect.Proxy;
 import java.util.Collections;
@@ -36,12 +37,14 @@
 import org.osgi.framework.BundleException;
 import org.osgi.framework.BundleListener;
 import org.osgi.framework.FrameworkListener;
+import org.osgi.framework.FrameworkUtil;
 import org.osgi.framework.InvalidSyntaxException;
 import org.osgi.framework.ServiceFactory;
 import org.osgi.framework.ServiceListener;
 import org.osgi.framework.ServiceObjects;
 import org.osgi.framework.ServiceReference;
 import org.osgi.framework.ServiceRegistration;
+import org.osgi.test.common.exceptions.Exceptions;
 
 public class CloseableBundleContext implements AutoCloseable, InvocationHandler {
 	private static final Consumer<ServiceRegistration<?>>	unregisterService	= asConsumerIgnoreException(
@@ -68,6 +71,13 @@ public class CloseableBundleContext implements AutoCloseable, InvocationHandler
 	private final Set<ServiceObjects<?>>					serviceobjects		= Collections
 		.synchronizedSet(Collections.newSetFromMap(new IdentityHashMap<>()));
 
+	public static BundleContext proxy(Class<?> host) {
+		return (BundleContext) Proxy.newProxyInstance(host.getClassLoader(), new Class<?>[] {
+			BundleContext.class, AutoCloseable.class
+		}, new CloseableBundleContext(host, FrameworkUtil.getBundle(host)
+			.getBundleContext()));
+	}
+
 	public static BundleContext proxy(Class<?> host, BundleContext bundleContext) {
 		return (BundleContext) Proxy.newProxyInstance(host.getClassLoader(), new Class<?>[] {
 			BundleContext.class, AutoCloseable.class
@@ -112,6 +122,11 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
 		throw new IllegalArgumentException();
 	}
 
+	public static void close(BundleContext bundleContext) {
+		CloseableBundleContext cbc = (CloseableBundleContext) Proxy.getInvocationHandler(bundleContext);
+		cbc.close();
+	}
+
 	@Override
 	public void close() {
 		bundles.stream()
@@ -238,7 +253,7 @@ public ClosableServiceObjects(ServiceObjects<S> so) {
 		@Override
 		public void close() {
 			instances.forEach((service, useCount) -> {
-				for (int i = 0; i < useCount; i++) {
+				for (int i = useCount; i > 0; i--) {
 					so.ungetService(service);
 				}
 			});
@@ -255,11 +270,15 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
 				.equals(ServiceObjects.class)) {
 
 				try {
-					Method ourMethod = getClass().getMethod(method.getName(), method.getParameterTypes());
-
-					return ourMethod.invoke(this, args);
-				} catch (NoSuchMethodException t) {
-					return method.invoke(so, args);
+					try {
+						Method ourMethod = getClass().getMethod(method.getName(), method.getParameterTypes());
+
+						return ourMethod.invoke(this, args);
+					} catch (NoSuchMethodException t) {
+						return method.invoke(so, args);
+					}
+				} catch (InvocationTargetException e) {
+					throw Exceptions.duck(e.getCause());
 				}
 			}
 			if (method.getDeclaringClass()
@@ -290,11 +309,14 @@ public S getService() {
 
 		@SuppressWarnings("unused")
 		public void ungetService(S service) {
-			// FIXME: need to decrement instance count rather than remove completely
-			instances.remove(service);
+			instances.compute(service, (key, oldValue) -> {
+				if (oldValue == null) {
+					throw new AssertionError("Attempt to ungetService " + service
+						+ " but there are no outstanding references to this object");
+				}
+				return oldValue == 1 ? null : oldValue - 1;
+			});
 			so.ungetService(service);
 		}
-
 	}
-
 }

org.osgi.test.common/src/main/java/org/osgi/test/common/install/InstallBundle.java

@@ -70,17 +70,14 @@ public Bundle installBundle(String pathToEmbeddedJar) {
 	 */
 	public Bundle installBundle(String pathToEmbeddedJar, boolean startBundle) {
 		int lastIndexOf = pathToEmbeddedJar.lastIndexOf('/');
-		String[] parts = new String[] {
+		String[] parts = (lastIndexOf == -1) ? new String[] {
 			"/", pathToEmbeddedJar
+		} : new String[] {
+			pathToEmbeddedJar.substring(0, lastIndexOf), pathToEmbeddedJar.substring(lastIndexOf + 1)
 		};
-		if (lastIndexOf != -1) {
-			parts = new String[] {
-				pathToEmbeddedJar.substring(0, lastIndexOf), pathToEmbeddedJar.substring(lastIndexOf + 1)
-			};
-		}
 		Enumeration<URL> entries = bundleContext.getBundle()
 			.findEntries(parts[0], parts[1], false);
-		if (!entries.hasMoreElements())
+		if (entries == null || !entries.hasMoreElements())
 			throw new AssertionError("No bundle entry " + pathToEmbeddedJar + " found in " + bundleContext.getBundle());
 		try (InputStream is = entries.nextElement()
 			.openStream()) {
@@ -94,4 +91,10 @@ public Bundle installBundle(String pathToEmbeddedJar, boolean startBundle) {
 		}
 	}
 
+	/**
+	 * @return The bundle context that this instance is attached to.
+	 */
+	public BundleContext getBundleContext() {
+		return bundleContext;
+	}
 }

org.osgi.test.common/src/main/java/org/osgi/test/common/service/ServiceConfiguration.java

@@ -77,7 +77,9 @@ public ServiceConfiguration<S> init(BundleContext bundleContext) {
 				.plusMillis(getTimeout());
 			if (!countDownLatch.await(getTimeout(), TimeUnit.MILLISECONDS)) {
 				throw new AssertionError(
-					getCardinality() + " services " + getFilter() + " didn't arrive within " + getTimeout() + "ms");
+					getCardinality() - tracker.size() + "/" + getCardinality() + " services " + getFilter()
+						+ " didn't arrive within "
+						+ getTimeout() + "ms");
 			}
 
 			// CountDownLatch is fired when the last addingService() is called,
@@ -89,7 +91,9 @@ public ServiceConfiguration<S> init(BundleContext bundleContext) {
 					.isAfter(endTime)) {
 
 					throw new AssertionError(
-						getCardinality() + " services " + getFilter() + " didn't arrive within " + getTimeout() + "ms");
+						getCardinality() - tracker.size() + "/" + getCardinality() + " services " + getFilter()
+							+ " didn't arrive within "
+							+ getTimeout() + "ms");
 				}
 				Thread.sleep(10);
 			}

org.osgi.test.common/src/test/java/org/osgi/test/common/context/CloseableBundleContextTest.java

@@ -0,0 +1,170 @@
+package org.osgi.test.common.context;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.reset;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
+import static org.mockito.Mockito.when;
+
+import org.assertj.core.api.Assertions;
+import org.assertj.core.api.SoftAssertions;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Nested;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.osgi.framework.BundleContext;
+import org.osgi.framework.ServiceObjects;
+import org.osgi.framework.ServiceReference;
+
+public class CloseableBundleContextTest extends SoftAssertions {
+	BundleContext			upstream;
+	BundleContext			sut;
+
+	@BeforeEach
+	void beforeEach() {
+		upstream = mock(BundleContext.class);
+		sut = CloseableBundleContext.proxy(CloseableBundleContextTest.class, upstream);
+	}
+
+	@ParameterizedTest
+	@ValueSource(strings = {
+		"hi", "there", "somethingElse"
+	})
+	void toString_showsDelegateInfo(String value) {
+		when(upstream.toString()).thenReturn(value);
+
+		assertThat(sut.toString()).contains(String.valueOf(System.identityHashCode(sut)))
+			.contains(value)
+			.startsWith(CloseableBundleContext.class.getSimpleName());
+	}
+
+	@Test
+	void hashCode_returnsUpstreamHashcode() {
+		assertThat(sut.hashCode()).isEqualTo(upstream.hashCode());
+	}
+
+	@Test
+	void equals_equalsUpstream() {
+		assertThat(sut).isEqualTo(upstream);
+	}
+
+	@Test
+	void implementsAutoCloseable() throws Exception {
+		Assertions.assertThat(sut)
+			.isInstanceOf(AutoCloseable.class);
+
+		// ((AutoCloseable) sut).close();
+	}
+
+	@Nested
+	class CloseableServiceObjectsTest {
+		ServiceObjects<Object>	upstreamSO;
+		ServiceObjects<Object>	sutSO;
+		AutoCloseable			sutSOCloseable;
+
+		final static String		s1	= "service1";
+		final static String		s2	= "service2";
+
+		@SuppressWarnings("unchecked")
+		@BeforeEach
+		void beforeEach() {
+			upstreamSO = mock(ServiceObjects.class);
+			when(upstreamSO.getService()).thenReturn(s1, s2, s1, s2);
+			when(upstream.getServiceObjects(any(ServiceReference.class))).thenReturn(upstreamSO);
+			sutSO = sut.getServiceObjects(mock(ServiceReference.class));
+			Assertions.assertThat(sutSO)
+				.isInstanceOf(AutoCloseable.class);
+			sutSOCloseable = (AutoCloseable) sutSO;
+		}
+
+		@SuppressWarnings("unchecked")
+		@Test
+		void getService_returnsServices_andClosesThem() throws Exception {
+			assertThat(sutSO.getService()).as("1")
+				.isSameAs(s1);
+			assertThat(sutSO.getService()).as("2")
+				.isSameAs(s2);
+			assertThat(sutSO.getService()).as("3")
+				.isSameAs(s1);
+			assertThat(sutSO.getService()).as("4")
+				.isSameAs(s2);
+			reset(upstreamSO);
+			sutSOCloseable.close();
+			check(() -> verify(upstreamSO, times(2)).ungetService(s1));
+			check(() -> verify(upstreamSO, times(2)).ungetService(s2));
+			check(() -> verifyNoMoreInteractions(upstreamSO));
+		}
+
+		@SuppressWarnings("unchecked")
+		@Test
+		void ungetService_decrementsUsage() throws Exception {
+			sutSO.getService();
+			sutSO.getService();
+			sutSO.getService();
+			sutSO.getService();
+			sutSO.ungetService(s1);
+			check(() -> verify(upstreamSO).ungetService(s1));
+			reset(upstreamSO);
+			sutSOCloseable.close();
+			check(() -> verify(upstreamSO).ungetService(s1));
+			check(() -> verify(upstreamSO, times(2)).ungetService(s2));
+			check(() -> verifyNoMoreInteractions(upstreamSO));
+		}
+
+		@Test
+		void ungetService_whenNoneGotten_asserts() {
+			assertThatCode(() -> sutSO.ungetService(s1)).isInstanceOf(AssertionError.class)
+				.hasMessageContaining(s1);
+		}
+
+		@Test
+		void ungetService_whenOneLeft_emptiesReferences() {
+			sutSO.getService();
+			sutSO.getService();
+			sutSO.getService();
+			assertThatCode(() -> sutSO.ungetService(s1)).doesNotThrowAnyException();
+			assertThatCode(() -> sutSO.ungetService(s1)).doesNotThrowAnyException();
+			assertThatCode(() -> sutSO.ungetService(s1)).isInstanceOf(AssertionError.class)
+				.hasMessageContaining(s1);
+		}
+
+		@ParameterizedTest
+		@ValueSource(strings = {
+			"hi", "there", "somethingElse"
+		})
+		void toString_showsDelegateInfo(String value) {
+			when(upstreamSO.toString()).thenReturn(value);
+
+			assertThat(sutSO.toString()).contains(String.valueOf(System.identityHashCode(sutSO)))
+				.contains(value)
+				.startsWith("CloseableServiceObjects");
+		}
+
+		@Test
+		void hashCode_returnsUpstreamHashcode() {
+			assertThat(sutSO.hashCode()).isEqualTo(upstreamSO.hashCode());
+		}
+
+		@Test
+		void equals_equalsUpstream() {
+			assertThat(sutSO).isEqualTo(upstreamSO);
+		}
+
+		@Test
+		void unmodifiedMethods_passedThrough() {
+			ServiceReference<Object> ref = mock(ServiceReference.class);
+			when(upstreamSO.getServiceReference()).thenReturn(ref);
+			assertThat(sutSO.getServiceReference()).isSameAs(ref);
+			check(() -> verify(upstreamSO).getServiceReference());
+		}
+	}
+
+	@AfterEach
+	void afterEach() {
+		assertAll();
+	}
+}

org.osgi.test.junit5/bnd/afterAll.bnd

@@ -0,0 +1 @@
+Export-Package: ${p}.tb1.*;-split-package:=first

org.osgi.test.junit5/bnd/afterClass.bnd

@@ -0,0 +1 @@
+Export-Package: ${p}.tb1.*;-split-package:=first

org.osgi.test.junit5/bnd/afterEach.bnd

@@ -0,0 +1 @@
+Export-Package: ${p}.tb1.*;-split-package:=first

org.osgi.test.junit5/bnd/beforeAll.bnd

@@ -0,0 +1 @@
+Export-Package: ${p}.tb1.*;-split-package:=first

org.osgi.test.junit5/bnd/beforeClass.bnd

@@ -0,0 +1 @@
+Export-Package: ${p}.tb1.*;-split-package:=first

org.osgi.test.junit5/bnd/beforeEach.bnd

@@ -0,0 +1 @@
+Export-Package: ${p}.tb1.*;-split-package:=first

org.osgi.test.junit5/bnd/innerTest.bnd

@@ -0,0 +1 @@
+Export-Package: ${p}.tb1.*;-split-package:=first

org.osgi.test.junit5/bnd/nested.afterEach.bnd

@@ -0,0 +1 @@
+Export-Package: ${p}.tb1.*;-split-package:=first

org.osgi.test.junit5/bnd/nested.beforeEach.bnd

@@ -0,0 +1 @@
+Export-Package: ${p}.tb1.*;-split-package:=first

org.osgi.test.junit5/bnd/nested.test.bnd

@@ -0,0 +1 @@
+Export-Package: ${p}.tb1.*;-split-package:=first

org.osgi.test.junit5/bnd/parameterizedTest.bnd

@@ -0,0 +1 @@
+Export-Package: ${p}.tb1.*;-split-package:=first