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

Failing build on Windows #4484

Closed
Barium opened this issue Sep 20, 2024 · 1 comment · Fixed by #4504
Closed

Failing build on Windows #4484

Barium opened this issue Sep 20, 2024 · 1 comment · Fixed by #4504
Labels
triage all new issues awaiting classification

Comments

@Barium
Copy link
Contributor

Barium commented Sep 20, 2024

Bug Report

When building on Windows some of the unit tests fail, I have previously mentioned this in a different issue, but have not had the time to create a fix for it. (See: #3408). Now I was returning to the Eclipse project and found that the build still failed on some unit tests. Mostly due to line ending issues. On Windows line endings do not have to be \r\n but can often times also be just \n. Often it depends on how you clone, or which editor you use. So the System.lineSeperator method will not be enough. Instead I would suggest using the Java \R linebreak matcher (See: https://docs.oracle.com/en/java/javase/11/docs/api/java.base/java/util/regex/Pattern.html#any_unicode_linebreak) as it matches all Unicode linebreak sequences.

I also found that the ClasspathReader method: classpathFor was called without modules, which caused the output to be the gradle default output. Adding a check catching for empty modules and exiting early, fixed this issue. Also windows paths can cause issues, but this is fixed by using: Paths.get(URI).toString() instead of URI.getPath().

Possible Implementation

I have attached the diff file of the changes I have made to resolve this issue, if you want I can create a pull request for it, else feel free to use it.

diff.patch:

index 2fa61df87..63e0198c8 100644
--- a/core/common/connector-core/src/test/java/org/eclipse/edc/connector/core/LocalPublicKeyDefaultExtensionTest.java
+++ b/core/common/connector-core/src/test/java/org/eclipse/edc/connector/core/LocalPublicKeyDefaultExtensionTest.java
@@ -26,6 +26,7 @@ import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.ExtendWith;
 
+import java.nio.file.Paths;
 import java.security.PublicKey;
 import java.util.Map;
 
@@ -73,7 +74,7 @@ class LocalPublicKeyDefaultExtensionTest {
         var value = TestUtils.getResourceFileContentAsString("rsa_2048.pem");
         var keys = Map.of(
                 "key1.id", "key1",
-                "key1.path", path.getPath());
+                "key1.path", Paths.get(path).toString());
 
         when(keyParserRegistry.parsePublic(value)).thenReturn(Result.success(mock(PublicKey.class)));
         when(context.getConfig(EDC_PUBLIC_KEYS_PREFIX)).thenReturn(ConfigFactory.fromMap(keys));
diff --git a/core/common/junit/src/main/java/org/eclipse/edc/junit/extensions/ClasspathReader.java b/core/common/junit/src/main/java/org/eclipse/edc/junit/extensions/ClasspathReader.java
index 97e6b72fb..e3e6638fc 100644
--- a/core/common/junit/src/main/java/org/eclipse/edc/junit/extensions/ClasspathReader.java
+++ b/core/common/junit/src/main/java/org/eclipse/edc/junit/extensions/ClasspathReader.java
@@ -40,6 +40,9 @@ public class ClasspathReader {
      */
     public static URL[] classpathFor(String... modules) {
         try {
+            if (modules.length == 0) {
+                return new URL[0];
+            }
             // Run a Gradle custom task to determine the runtime classpath of the module to run
             var printClasspath = Arrays.stream(modules).map(it -> it + ":printClasspath");
             var commandStream = Stream.of(GRADLE_WRAPPER.getCanonicalPath(), "-q");
diff --git a/core/common/lib/keys-lib/src/main/java/org/eclipse/edc/keys/VaultCertificateResolver.java b/core/common/lib/keys-lib/src/main/java/org/eclipse/edc/keys/VaultCertificateResolver.java
index 3615fc1ec..ca311c06b 100644
--- a/core/common/lib/keys-lib/src/main/java/org/eclipse/edc/keys/VaultCertificateResolver.java
+++ b/core/common/lib/keys-lib/src/main/java/org/eclipse/edc/keys/VaultCertificateResolver.java
@@ -47,7 +47,7 @@ public class VaultCertificateResolver implements CertificateResolver {
         }
 
         try {
-            var encoded = certificateRepresentation.replace(HEADER, "").replaceAll(System.lineSeparator(), "").replace(FOOTER, "");
+            var encoded = certificateRepresentation.replace(HEADER, "").replaceAll("\\R", "").replace(FOOTER, "");
             CertificateFactory fact = CertificateFactory.getInstance("X.509");
             return (X509Certificate) fact.generateCertificate(new ByteArrayInputStream(Base64.getDecoder().decode(encoded.getBytes())));
         } catch (GeneralSecurityException | IllegalArgumentException e) {
diff --git a/core/common/lib/keys-lib/src/test/java/org/eclipse/edc/keys/VaultCertificateResolverTest.java b/core/common/lib/keys-lib/src/test/java/org/eclipse/edc/keys/VaultCertificateResolverTest.java
index 58c077444..55ad474e3 100644
--- a/core/common/lib/keys-lib/src/test/java/org/eclipse/edc/keys/VaultCertificateResolverTest.java
+++ b/core/common/lib/keys-lib/src/test/java/org/eclipse/edc/keys/VaultCertificateResolverTest.java
@@ -17,6 +17,7 @@ package org.eclipse.edc.keys;
 import org.eclipse.edc.spi.EdcException;
 import org.eclipse.edc.spi.security.Vault;
 import org.jetbrains.annotations.NotNull;
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 
@@ -58,7 +59,7 @@ class VaultCertificateResolverTest {
         var pemReceived = convertCertificateToPem(certificate);
 
         verify(vault, times(1)).resolveSecret(KEY);
-        assertThat(pemReceived).isEqualTo(pemExpected);
+        Assertions.assertArrayEquals(pemExpected.split("\\R"), pemReceived.split("\\R"));
     }
 
     @Test
diff --git a/extensions/common/iam/oauth2/oauth2-core/src/test/java/org/eclipse/edc/iam/oauth2/jwt/X509CertificateDecoratorTest.java b/extensions/common/iam/oauth2/oauth2-core/src/test/java/org/eclipse/edc/iam/oauth2/jwt/X509CertificateDecoratorTest.java
index ccb3ae7fe..7fcba868f 100644
--- a/extensions/common/iam/oauth2/oauth2-core/src/test/java/org/eclipse/edc/iam/oauth2/jwt/X509CertificateDecoratorTest.java
+++ b/extensions/common/iam/oauth2/oauth2-core/src/test/java/org/eclipse/edc/iam/oauth2/jwt/X509CertificateDecoratorTest.java
@@ -36,7 +36,7 @@ class X509CertificateDecoratorTest {
     private static X509Certificate createCertificate() throws CertificateException, IOException {
         var classloader = Thread.currentThread().getContextClassLoader();
         var pem = new String(Objects.requireNonNull(classloader.getResourceAsStream(TEST_CERT_FILE)).readAllBytes());
-        var encoded = pem.replace(HEADER, "").replaceAll(System.lineSeparator(), "").replace(FOOTER, "");
+        var encoded = pem.replace(HEADER, "").replaceAll("\\R", "").replace(FOOTER, "");
         CertificateFactory fact = CertificateFactory.getInstance("X.509");
         return (X509Certificate) fact.generateCertificate(new ByteArrayInputStream(Base64.getDecoder().decode(encoded.getBytes())));
     }
@github-actions github-actions bot added the triage all new issues awaiting classification label Sep 20, 2024
@paullatzelsperger
Copy link
Member

hi @Barium, looks like a clean patch. If you change your assertions to AssertJ assertions, you are good to go for a PR!

Barium added a commit to Barium/Connector that referenced this issue Sep 27, 2024
Change path handling to correctly handle Windows pathnames.

Change line ending handling from using `System.lineSeparator` to using
the regex \R to treat all unicode line ending character sequences as the
line endings.

Change unit tests that compared certificates read from file, to ignore
the line ending characters, so differences in line ending did not break
the test.

Refs: eclipse-edc#4484
paullatzelsperger pushed a commit that referenced this issue Sep 27, 2024
Change path handling to correctly handle Windows pathnames.

Change line ending handling from using `System.lineSeparator` to using
the regex \R to treat all unicode line ending character sequences as the
line endings.

Change unit tests that compared certificates read from file, to ignore
the line ending characters, so differences in line ending did not break
the test.

Refs: #4484
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
triage all new issues awaiting classification
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants