From 8d386590f4ece1e8ea13ea5ecad3dba1f9bce030 Mon Sep 17 00:00:00 2001 From: Andre Weber Date: Thu, 30 Nov 2023 13:52:28 +0100 Subject: [PATCH 1/5] feature: Run Integration Tests on GitHub Actions Closes: #41 Signed-off-by: Andre Weber --- .github/workflows/build.yaml | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index ac6dffc2..8ea063a7 100755 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -11,6 +11,9 @@ jobs: - name: Checkout code uses: actions/checkout@v3 + - name: "Run Databroker" + run: docker run --pull=always --rm --publish 55556:55556/tcp --detach ghcr.io/eclipse/kuksa.val/databroker:master --port 55556 --insecure + - uses: actions/setup-java@v3 with: distribution: 'temurin' @@ -26,4 +29,4 @@ jobs: run: ./gradlew ktlintCheck detekt - name: Run 'test' with Gradle Wrapper - run: ./gradlew test -Dkotest.tags="!Integration" + run: ./gradlew test -Dkotest.tags="!Secure" From b972b1bc5101ba08a507f7afc88a1b5fa6ade1fa Mon Sep 17 00:00:00 2001 From: Andre Weber Date: Fri, 1 Dec 2023 09:55:28 +0100 Subject: [PATCH 2/5] chore: Fix Failing Tests --- .../SpecificationPropertyListener.kt | 4 +- .../eclipse/kuksa/DataBrokerConnectionTest.kt | 40 ++++++------ .../DataBrokerTransporterExtensions.kt | 14 +++- .../subscription/DataBrokerSubscriberTest.kt | 65 +++++++++++++------ .../kotlin/org/integration-tests-guide.md | 32 +++++++++ 5 files changed, 109 insertions(+), 46 deletions(-) create mode 100644 kuksa-sdk/src/test/kotlin/org/integration-tests-guide.md diff --git a/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/subscription/SpecificationPropertyListener.kt b/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/subscription/SpecificationPropertyListener.kt index 7282888b..4de50f5d 100644 --- a/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/subscription/SpecificationPropertyListener.kt +++ b/kuksa-sdk/src/main/kotlin/org/eclipse/kuksa/subscription/SpecificationPropertyListener.kt @@ -75,9 +75,7 @@ internal class SpecificationPropertyListener( other as SpecificationPropertyListener<*> - if (listener != other.listener) return false - - return true + return listener == other.listener } override fun hashCode(): Int { diff --git a/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/DataBrokerConnectionTest.kt b/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/DataBrokerConnectionTest.kt index bfd15788..9a936ebd 100644 --- a/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/DataBrokerConnectionTest.kt +++ b/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,7 +72,9 @@ class DataBrokerConnectionTest : BehaviorSpec({ then("The #onPropertyChanged callback is triggered with the new value") { val capturingSlot = slot() - verify { propertyListener.onPropertyChanged(any(), any(), capture(capturingSlot)) } + verify(timeout = 100) { + propertyListener.onPropertyChanged(any(), any(), capture(capturingSlot)) + } val dataEntry = capturingSlot.captured val capturedDatapoint = dataEntry.value @@ -78,16 +82,6 @@ class DataBrokerConnectionTest : BehaviorSpec({ 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>(relaxed = true) + val specificationListener = + mockk>(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() + val capturingList = mutableListOf() - 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() - 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 diff --git a/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/extensions/DataBrokerTransporterExtensions.kt b/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/extensions/DataBrokerTransporterExtensions.kt index ce0a3adb..00678135 100644 --- a/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/extensions/DataBrokerTransporterExtensions.kt +++ b/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/extensions/DataBrokerTransporterExtensions.kt @@ -24,14 +24,19 @@ 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") } @@ -39,7 +44,10 @@ internal suspend fun DataBrokerTransporter.updateRandomFloatValue(vssPath: Strin 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() diff --git a/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/subscription/DataBrokerSubscriberTest.kt b/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/subscription/DataBrokerSubscriberTest.kt index c5e87207..44c86f03 100644 --- a/kuksa-sdk/src/test/kotlin/org/eclipse/kuksa/subscription/DataBrokerSubscriberTest.kt +++ b/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,10 +78,10 @@ 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()) } } @@ -89,8 +89,6 @@ class DataBrokerSubscriberTest : BehaviorSpec({ } `when`("Subscribing multiple (different) PropertyListener to $vssPath") { - clearMocks(propertyListener) - val propertyListenerMocks = mutableListOf() repeat(10) { val otherPropertyListenerMock = mockk(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() - 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() - 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>(relaxed = true) - classUnderTest.subscribe(specification, Types.Field.FIELD_VALUE, specificationObserverMock) + val specificationObserverMock = + mockk>(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() + 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>(relaxed = true) - classUnderTest.subscribe(specification, Types.Field.FIELD_VALUE, specificationObserverMock) - classUnderTest.subscribe(specification, Types.Field.FIELD_VALUE, specificationObserverMock) + val specificationObserverMock = + mockk>(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() - 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 diff --git a/kuksa-sdk/src/test/kotlin/org/integration-tests-guide.md b/kuksa-sdk/src/test/kotlin/org/integration-tests-guide.md new file mode 100644 index 00000000..ed2f35e3 --- /dev/null +++ b/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() + + 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 +``` From 31a44a560a9681b859ca04a5a30c583a63ec7056 Mon Sep 17 00:00:00 2001 From: Andre Weber Date: Fri, 1 Dec 2023 11:14:13 +0100 Subject: [PATCH 3/5] chore: Stop Docker Databroker Container after use --- .github/workflows/build.yaml | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index 8ea063a7..e8324c0d 100755 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -11,8 +11,8 @@ jobs: - name: Checkout code uses: actions/checkout@v3 - - name: "Run Databroker" - run: docker run --pull=always --rm --publish 55556:55556/tcp --detach ghcr.io/eclipse/kuksa.val/databroker:master --port 55556 --insecure + - name: "Run 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: @@ -30,3 +30,7 @@ jobs: - name: Run 'test' with Gradle Wrapper run: ./gradlew test -Dkotest.tags="!Secure" + + - name: "Stop Databroker" + if: always() + run: docker stop databroker From 6b88d1ac5e411005029c972a3a31f05e59be1b18 Mon Sep 17 00:00:00 2001 From: Andre Weber Date: Fri, 1 Dec 2023 12:05:20 +0100 Subject: [PATCH 4/5] chore: Add Reports to Build Pipeline chore: Minor Changes --- .github/workflows/build.yaml | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/.github/workflows/build.yaml b/.github/workflows/build.yaml index e8324c0d..3b2f9429 100755 --- a/.github/workflows/build.yaml +++ b/.github/workflows/build.yaml @@ -11,7 +11,7 @@ jobs: - name: Checkout code uses: actions/checkout@v3 - - name: "Run Databroker in detached mode" + - 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 @@ -25,12 +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="!Secure" - - name: "Stop Databroker" + - 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 From 79eb2c0c730127ce5581ea9134fd134f8e1af875 Mon Sep 17 00:00:00 2001 From: Andre Weber Date: Mon, 4 Dec 2023 08:34:25 +0100 Subject: [PATCH 5/5] chore: Fix .editorconfig --- .editorconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.editorconfig b/.editorconfig index cc308c2f..d5db151e 100644 --- a/.editorconfig +++ b/.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