Merge pull request #55 from boschglobal/feature-41

Run Integration Tests on GitHub Actions

.editorconfig

@@ -8,7 +8,7 @@ root = true
 end_of_line = lf
 insert_final_newline = true
 
-[*.{java, kt, kts, xml}]
+[*.{java,kt,kts,xml}]
 charset = utf-8
 trim_trailing_whitespace = true
 indent_style = space

.github/workflows/build.yaml

@@ -11,6 +11,9 @@ jobs:
       - name: Checkout code
         uses: actions/checkout@v3
 
+      - name: "Run Docker Container of Databroker in detached mode"
+        run: docker run --pull=always --rm --publish 55556:55556/tcp --detach --name databroker ghcr.io/eclipse/kuksa.val/databroker:master --port 55556 --insecure
+
       - uses: actions/setup-java@v3
         with:
           distribution: 'temurin'
@@ -22,8 +25,28 @@ jobs:
       - name: Run 'assemble' with Gradle Wrapper
         run: ./gradlew assemble
 
-      - name: Run 'check' with Gradle Wrapper
+      - name: Run 'lint' with Gradle Wrapper
         run: ./gradlew ktlintCheck detekt
 
+      - name: Upload Detekt Reports
+        uses: actions/upload-artifact@v3
+        with:
+          name: detekt-reports
+          path: ${{ github.workspace }}/build/reports/detekt
+          if-no-files-found: error
+          retention-days: 14
+
       - name: Run 'test' with Gradle Wrapper
-        run: ./gradlew test -Dkotest.tags="!Integration"
+        run: ./gradlew test -Dkotest.tags="!Secure"
+
+      - name: Upload Test Reports
+        uses: actions/upload-artifact@v3
+        with:
+          name: test-reports
+          path: ${{ github.workspace }}/**/reports/tests/*/
+          if-no-files-found: error
+          retention-days: 14
+
+      - name: "Stop Docker Container of Databroker"
+        if: always()
+        run: docker stop databroker

kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/subscription/SpecificationPropertyListener.kt

@@ -75,9 +75,7 @@ internal class SpecificationPropertyListener<T : VssSpecification>(
 
         other as SpecificationPropertyListener<*>
 
-        if (listener != other.listener) return false
-
-        return true
+        return listener == other.listener
     }
 
     override fun hashCode(): Int {

kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/DataBrokerConnectionTest.kt

@@ -56,7 +56,9 @@ class DataBrokerConnectionTest : BehaviorSpec({
                 dataBrokerConnection.subscribe(property, propertyListener)
 
                 then("The #onPropertyChanged method is triggered") {
-                    verify(timeout = 100L) { propertyListener.onPropertyChanged(any(), any(), any()) }
+                    verify(timeout = 100L) {
+                        propertyListener.onPropertyChanged(any(), any(), any())
+                    }
                 }
 
                 `when`("The observed Property changes") {
@@ -70,24 +72,16 @@ class DataBrokerConnectionTest : BehaviorSpec({
                     then("The #onPropertyChanged callback is triggered with the new value") {
                         val capturingSlot = slot<Types.DataEntry>()
 
-                        verify { propertyListener.onPropertyChanged(any(), any(), capture(capturingSlot)) }
+                        verify(timeout = 100) {
+                            propertyListener.onPropertyChanged(any(), any(), capture(capturingSlot))
+                        }
 
                         val dataEntry = capturingSlot.captured
                         val capturedDatapoint = dataEntry.value
                         val float = capturedDatapoint.float
 
                         assertEquals(newValue, float, 0.0001f)
                     }
-
-                    `when`("The same value is set again") {
-                        clearMocks(propertyListener)
-
-                        dataBrokerConnection.update(property, datapoint)
-
-                        then("The #onPropertyChangedCallback should NOT be triggered again") {
-                            verify(exactly = 0) { propertyListener.onPropertyChanged(any(), any(), any()) }
-                        }
-                    }
                 }
             }
 
@@ -160,11 +154,15 @@ class DataBrokerConnectionTest : BehaviorSpec({
             }
 
             `when`("Subscribing to the specification") {
-                val specificationListener = mockk<VssSpecificationListener<VssDriver>>(relaxed = true)
+                val specificationListener =
+                    mockk<VssSpecificationListener<VssDriver>>(relaxed = true)
                 dataBrokerConnection.subscribe(specification, listener = specificationListener)
 
                 then("The #onSpecificationChanged method is triggered") {
-                    verify(timeout = 100L) { specificationListener.onSpecificationChanged(any()) }
+                    verify(
+                        timeout = 100L,
+                        exactly = 1,
+                    ) { specificationListener.onSpecificationChanged(any()) }
                 }
 
                 and("The initial value is different from the default for a child") {
@@ -174,11 +172,13 @@ class DataBrokerConnectionTest : BehaviorSpec({
                     dataBrokerConnection.update(property, datapoint)
 
                     then("Every child property has been updated with the correct value") {
-                        val capturingSlots = mutableListOf<VssDriver>()
+                        val capturingList = mutableListOf<VssDriver>()
 
-                        verify(exactly = 2) { specificationListener.onSpecificationChanged(capture(capturingSlots)) }
+                        verify(timeout = 100, exactly = 2) {
+                            specificationListener.onSpecificationChanged(capture(capturingList))
+                        }
 
-                        val updatedDriver = capturingSlots[1]
+                        val updatedDriver = capturingList.last()
                         val heartRate = updatedDriver.heartRate
 
                         heartRate.value shouldBe newHeartRateValue
@@ -194,9 +194,11 @@ class DataBrokerConnectionTest : BehaviorSpec({
                     then("The subscribed Specification should be updated") {
                         val capturingSlots = mutableListOf<VssDriver>()
 
-                        verify(exactly = 3) { specificationListener.onSpecificationChanged(capture(capturingSlots)) }
+                        verify(timeout = 100, exactly = 3) {
+                            specificationListener.onSpecificationChanged(capture(capturingSlots))
+                        }
 
-                        val updatedDriver = capturingSlots[2]
+                        val updatedDriver = capturingSlots.last()
                         val heartRate = updatedDriver.heartRate
 
                         heartRate.value shouldBe newHeartRateValue

kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/extensions/DataBrokerTransporterExtensions.kt

@@ -24,22 +24,30 @@ import org.eclipse.kuksa.DataBrokerTransporter
 import org.eclipse.kuksa.proto.v1.Types
 import kotlin.random.Random
 
-internal suspend fun DataBrokerTransporter.updateRandomFloatValue(vssPath: String, maxValue: Int = 300): Float {
+internal suspend fun DataBrokerTransporter.updateRandomFloatValue(
+    vssPath: String,
+    maxValue: Int = 300,
+): Float {
     val random = Random(System.nanoTime())
     val randomValue = random.nextInt(maxValue)
     val randomFloat = randomValue.toFloat()
     val updatedDatapoint = Types.Datapoint.newBuilder().setFloat(randomFloat).build()
 
+    val fields = listOf(Types.Field.FIELD_VALUE)
+
     try {
-        update(vssPath, listOf(Types.Field.FIELD_VALUE), updatedDatapoint)
+        update(vssPath, fields, updatedDatapoint)
     } catch (e: Exception) {
         fail("Updating $vssPath to $randomFloat failed: $e")
     }
 
     return randomFloat
 }
 
-internal suspend fun DataBrokerTransporter.updateRandomUint32Value(vssPath: String, maxValue: Int = 300): Int {
+internal suspend fun DataBrokerTransporter.updateRandomUint32Value(
+    vssPath: String,
+    maxValue: Int = 300,
+): Int {
     val random = Random(System.nanoTime())
     val randomValue = random.nextInt(maxValue)
     val updatedDatapoint = Types.Datapoint.newBuilder().setUint32(randomValue).build()

kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/subscription/DataBrokerSubscriberTest.kt

@@ -25,6 +25,7 @@ import io.mockk.clearMocks
 import io.mockk.every
 import io.mockk.mockk
 import io.mockk.verify
+import kotlinx.coroutines.delay
 import org.eclipse.kuksa.DataBrokerTransporter
 import org.eclipse.kuksa.PropertyListener
 import org.eclipse.kuksa.VssSpecificationListener
@@ -48,7 +49,8 @@ class DataBrokerSubscriberTest : BehaviorSpec({
         connector.connect()
 
         and("An Instance of DataBrokerSubscriber") {
-            val databrokerTransporter = DataBrokerTransporter(dataBrokerConnectorProvider.managedChannel)
+            val databrokerTransporter =
+                DataBrokerTransporter(dataBrokerConnectorProvider.managedChannel)
             val classUnderTest = DataBrokerSubscriber(databrokerTransporter)
 
             `when`("Subscribing using VSS_PATH to Vehicle.Speed with FIELD_VALUE") {
@@ -61,15 +63,13 @@ class DataBrokerSubscriberTest : BehaviorSpec({
                     databrokerTransporter.updateRandomFloatValue(vssPath)
 
                     then("The PropertyListener is notified about the change") {
-                        verify(timeout = 100L) {
+                        verify(timeout = 100L, exactly = 2) {
                             propertyListener.onPropertyChanged(vssPath, fieldValue, any())
                         }
                     }
                 }
 
                 `when`("Subscribing the same PropertyListener to a different vssPath") {
-                    clearMocks(propertyListener)
-
                     val otherVssPath = "Vehicle.ADAS.CruiseControl.SpeedSet"
                     classUnderTest.subscribe(otherVssPath, fieldValue, propertyListener)
 
@@ -78,19 +78,17 @@ class DataBrokerSubscriberTest : BehaviorSpec({
                         databrokerTransporter.updateRandomFloatValue(otherVssPath)
 
                         then("The Observer is notified about both changes") {
-                            verify(timeout = 100L) {
+                            verify(timeout = 100L, exactly = 3) {
                                 propertyListener.onPropertyChanged(vssPath, fieldValue, any())
                             }
-                            verify(timeout = 100L) {
+                            verify(timeout = 100L, exactly = 2) {
                                 propertyListener.onPropertyChanged(otherVssPath, fieldValue, any())
                             }
                         }
                     }
                 }
 
                 `when`("Subscribing multiple (different) PropertyListener to $vssPath") {
-                    clearMocks(propertyListener)
-
                     val propertyListenerMocks = mutableListOf<PropertyListener>()
                     repeat(10) {
                         val otherPropertyListenerMock = mockk<PropertyListener>(relaxed = true)
@@ -102,11 +100,11 @@ class DataBrokerSubscriberTest : BehaviorSpec({
                     and("When the FIELD_VALUE of Vehicle.Speed is updated") {
                         val randomFloatValue = databrokerTransporter.updateRandomFloatValue(vssPath)
 
-                        then("The PropertyListener is only notified once") {
+                        then("Each PropertyListener is only notified once") {
                             propertyListenerMocks.forEach { propertyListenerMock ->
                                 val dataEntries = mutableListOf<DataEntry>()
 
-                                verify(timeout = 100L) {
+                                verify(timeout = 100L, exactly = 2) {
                                     propertyListenerMock.onPropertyChanged(vssPath, fieldValue, capture(dataEntries))
                                 }
 
@@ -123,9 +121,10 @@ class DataBrokerSubscriberTest : BehaviorSpec({
 
                     and("When the FIELD_VALUE of Vehicle.Speed is updated") {
                         databrokerTransporter.updateRandomFloatValue(vssPath)
+                        delay(100)
 
                         then("The PropertyListener is not notified") {
-                            verify(timeout = 100L, exactly = 0) {
+                            verify(exactly = 0) {
                                 propertyListener.onPropertyChanged(vssPath, fieldValue, any())
                             }
                         }
@@ -146,7 +145,7 @@ class DataBrokerSubscriberTest : BehaviorSpec({
                     then("The PropertyListener is only notified once") {
                         val dataEntries = mutableListOf<DataEntry>()
 
-                        verify(timeout = 100L) {
+                        verify(timeout = 100L, exactly = 2) {
                             propertyListenerMock.onPropertyChanged(vssPath, fieldValue, capture(dataEntries))
                         }
 
@@ -159,30 +158,54 @@ class DataBrokerSubscriberTest : BehaviorSpec({
             val specification = VssDriver.VssHeartRate()
 
             `when`("Subscribing using VssSpecification to Vehicle.Driver.HeartRate with Field FIELD_VALUE") {
-                val specificationObserverMock = mockk<VssSpecificationListener<VssDriver.VssHeartRate>>(relaxed = true)
-                classUnderTest.subscribe(specification, Types.Field.FIELD_VALUE, specificationObserverMock)
+                val specificationObserverMock =
+                    mockk<VssSpecificationListener<VssDriver.VssHeartRate>>(relaxed = true)
+                classUnderTest.subscribe(
+                    specification,
+                    Types.Field.FIELD_VALUE,
+                    specificationObserverMock,
+                )
 
                 and("The value of Vehicle.Driver.HeartRate changes") {
-                    databrokerTransporter.updateRandomFloatValue(specification.vssPath)
+                    val randomIntValue =
+                        databrokerTransporter.updateRandomUint32Value(specification.vssPath)
 
                     then("The Observer should be triggered") {
-                        verify(timeout = 100) { specificationObserverMock.onSpecificationChanged(any()) }
+                        val vssHeartRates = mutableListOf<VssDriver.VssHeartRate>()
+                        verify(timeout = 100, exactly = 2) {
+                            specificationObserverMock.onSpecificationChanged(capture(vssHeartRates))
+                        }
+
+                        val count = vssHeartRates.count { it.value == randomIntValue }
+                        count shouldBe 1
                     }
                 }
             }
 
             `when`("Subscribing the same SpecificationObserver twice to Vehicle.Driver.HeartRate") {
-                val specificationObserverMock = mockk<VssSpecificationListener<VssDriver.VssHeartRate>>(relaxed = true)
-                classUnderTest.subscribe(specification, Types.Field.FIELD_VALUE, specificationObserverMock)
-                classUnderTest.subscribe(specification, Types.Field.FIELD_VALUE, specificationObserverMock)
+                val specificationObserverMock =
+                    mockk<VssSpecificationListener<VssDriver.VssHeartRate>>(relaxed = true)
+                classUnderTest.subscribe(
+                    specification,
+                    Types.Field.FIELD_VALUE,
+                    specificationObserverMock,
+                )
+                classUnderTest.subscribe(
+                    specification,
+                    Types.Field.FIELD_VALUE,
+                    specificationObserverMock,
+                )
 
                 and("The value of Vehicle.Driver.HeartRate changes") {
-                    val randomIntValue = databrokerTransporter.updateRandomUint32Value(specification.vssPath)
+                    val randomIntValue =
+                        databrokerTransporter.updateRandomUint32Value(specification.vssPath)
 
                     then("The Observer is only notified once") {
                         val heartRates = mutableListOf<VssDriver.VssHeartRate>()
 
-                        verify(timeout = 100) { specificationObserverMock.onSpecificationChanged(capture(heartRates)) }
+                        verify(timeout = 100, exactly = 2) {
+                            specificationObserverMock.onSpecificationChanged(capture(heartRates))
+                        }
 
                         val count = heartRates.count { it.value == randomIntValue }
                         count shouldBe 1

kuksa-sdk/src/test/kotlin/org/integration-tests-guide.md

@@ -0,0 +1,32 @@
+While DataBrokerConnection#fetch and DataBrokerConnection#update are running synchronously the observers of the
+corresponding SubscriptionChannels run asynchronously. Between the update executed using #updateRandomXXXValue
+here and the propagation to the subscribed observers a small timeframe may pass. Therefore things like this most
+likely won't work because of the behavior described above:
+```
+    subscribe("Vehicle.Speed", observer)
+    updateRandomFloatValue("Vehicle.Speed")
+    verify { observer.onPropertyChanged("Vehicle.Speed", any())}
+```
+
+Keep in mind, that when calling DataBrokerConnection#subscribe an initial update with the current value is send,
+to the subscribing observer, meaning that checking for a successful update requires at least two calls to the
+#onPropertyChanged method. One for the initial update and one for the consecutive, real update of the value.
+
+Using verify(timeout = 100\[, exactly = 1\]) alone won't fix this issue, because "exactly = 1" will always be 
+fulfilled by the initial update mentioned above. It therefore needs to be something like 
+verify(timeout = 100, exactly = 2) if an update is expected.
+
+A better example therefore would be:
+```
+    subscribe("Vehicle.Speed", observer) // triggers first #onPropertyChanged with initial value
+    val value = updateRandomFloatValue() // triggers second #onPropertyChanged with updated value
+
+    val dataEntries = mutableListOf<DataEntry>()
+
+    verify (timeout = 100, exactly = 2) { // initial update +  "updateRandomFloatValue")
+        observer.onPropertyChanged("Vehicle.Speed", capture(dataEntries))
+    }
+
+    val count = dataEntries.count { it.value.float == randomFloatValue }
+    count shouldBe 1
+```