Copilot commented on code in PR #45:
URL:
https://github.com/apache/sling-org-apache-sling-auth-oauth-client/pull/45#discussion_r3118882644
##########
src/test/java/org/apache/sling/auth/oauth_client/AuthorizationCodeFlowIT.java:
##########
@@ -127,10 +128,16 @@ void initKeycloak() {
// this is most usually done in an IDE, with both Keycloak and Sling
running
String existingKeyCloakUrl = System.getenv("KEYCLOAK_URL");
if (existingKeyCloakUrl == null) {
- keycloak = new KeycloakContainer("quay.io/keycloak/keycloak:26.4")
- .withRealmImportFile("keycloak-import/sling.json");
- keycloak.start();
- keycloakPort = keycloak.getHttpPort();
+ try {
+ keycloak = new
KeycloakContainer("quay.io/keycloak/keycloak:26.4")
+ .withRealmImportFile("keycloak-import/sling.json");
+ keycloak.start();
+ keycloakPort = keycloak.getHttpPort();
+ } catch (RuntimeException e) {
+ Assumptions.assumeTrue(
+ false,
+ "Skipping integration test: Keycloak test container is
unavailable (" + e.getMessage() + ")");
+ }
Review Comment:
The try/catch around `keycloak.start()` catches any `RuntimeException` and
converts it into a skipped test. This can mask genuine test/setup regressions
(e.g., realm import/resource issues) by reporting them as “container
unavailable”. Consider narrowing the catch to the specific
Testcontainers/Docker-availability exceptions (or pre-checking Docker
availability) and rethrowing unexpected runtime failures so real breakages
still fail the build.
##########
pom.xml:
##########
@@ -78,6 +78,8 @@
<dependency>
<groupId>org.slf4j</groupId>
<artifactId>slf4j-api</artifactId>
+ <version>1.7.36</version>
+ <!-- version from Starter 13 -->
Review Comment:
Pinning only `slf4j-api` to 1.7.36 while leaving `slf4j-simple` (test) to be
managed by the parent can create an API/binding version mismatch on the test
classpath if the parent updates the binding version. To keep SLF4J artifacts
consistent, consider introducing a single `slf4j.version` property (or
dependencyManagement) and applying it to both `slf4j-api` and any SLF4J
bindings used in tests.
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
