Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] Upgrade Karaf from 4.4.6 to 4.4.7 #17515

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

holgerfriedrich
Copy link
Member

Still WIP. Depends on Karaf 4.4.7 release.
Relates to openhab/openhab-core#4406.
I updated a few commons libs in the addons as well to keep it in sync with core. Hope this does not break something.

  • Sync runtime dependencies with Karaf 4.4.7, most notably:
    • BouncyCastle 1.78.1
    • CXF 3.6.4
    • Jackson 2.17.2
    • JNA 5.15.0
    • JAXB 2.3.9
    • commons-io 2.17.0
    • commons-lang3 3.17.0
    • commons-logging 1.3.4
  • Resolve itest runbundles

Copy link
Member

@david-pace david-pace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for boschshc 👍

Copy link
Contributor

@ccutrer ccutrer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM for mqtt.homeassistant and jinja

Copy link
Member

@david-pace david-pace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found an assertion that should be slightly adapted in our boschshc tests.

@@ -278,7 +278,7 @@ void sendRequestInvalidSyntaxInResponse()
ExecutionException e = assertThrows(ExecutionException.class,
() -> httpClient.sendRequest(request, SubscribeResult.class, sr -> false, null));
assertEquals(
"Received invalid content in response, expected type org.openhab.binding.boschshc.internal.devices.bridge.dto.SubscribeResult: com.google.gson.stream.MalformedJsonException: Unterminated string at line 1 column 44 path $.@type",
"Received invalid content in response, expected type org.openhab.binding.boschshc.internal.devices.bridge.dto.SubscribeResult: com.google.gson.stream.MalformedJsonException: Unterminated string at line 1 column 44 path $.@type\nSee https://github.com/google/gson/blob/main/Troubleshooting.md#malformed-json",
Copy link
Member

@david-pace david-pace Jan 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking at this assertion again, I realize that only the first part comes from our openHAB code. We should not test messages from external libraries since this makes our tests unstable. Therefore I would propose to test only that the exception message starts with our message:

assertThat(e.getMessage(), startsWith(
                "Received invalid content in response, expected type org.openhab.binding.boschshc.internal.devices.bridge.dto.SubscribeResult:"));

This check uses Hamcrest assertions as recommended in the unit test guidelines. The corresponding imports are:

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.core.StringStartsWith.startsWith;

* Sync runtime dependencies with Karaf 4.4.7, most notably:
   * PaxWeb 8.0.30
   * Jetty 9.4.57.v20241219
   * BouncyCastle 1.78.1
   * CXF 3.6.4
   * Jackson 2.18.2
   * JNA 5.16.0
   * JAXB 2.3.9
   * commons-io 2.17.0
   * commons-lang3 3.17.0
   * commons-logging 1.3.4
   * ASM 9.7.1
   * PaxLogging 2.2.8
 * Resolve itest runbundles

Signed-off-by: Holger Friedrich <[email protected]>
* gson 2.11.0
* guava 3.33.1
* ecj 3.39.0
* Fix newly introduced compilation and test issues in bindings

Signed-off-by: Holger Friedrich <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting external dependency Depends on a new version of an external dependency work in progress A PR that is not yet ready to be merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants