Issue #5965 - fixing invalid dot output from start.jar (#7606)

* Adding testcase that can use `dot` if it exists on the machine.
jetty · Oct 6, 2022 · 660f601 · 660f601
1 parent 281f0dc
commit 660f601
Show file tree

Hide file tree

Showing 2 changed files with 60 additions and 8 deletions.
diff --git a/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/ModuleGraphWriter.java b/jetty-core/jetty-start/src/main/java/org/eclipse/jetty/start/ModuleGraphWriter.java
@@ -29,6 +29,8 @@ public class ModuleGraphWriter
 {
     private String colorModuleBg;
     private String colorEnabledBg;
+    private String colorEdgeBefore;
+    private String colorEdgeAfter;
     private String colorTransitiveBg;
     private String colorCellBg;
     private String colorHeaderBg;
@@ -38,6 +40,8 @@ public ModuleGraphWriter()
     {
         colorModuleBg = "#B8FFB8";
         colorEnabledBg = "#66FFCC";
+        colorEdgeAfter = "#00CC33";
+        colorEdgeAfter = "#33CC00";
         colorTransitiveBg = "#66CC66";
         colorCellBg = "#FFFFFF80";
         colorHeaderBg = "#00000020";
@@ -49,6 +53,8 @@ public void config(Props props)
         String prefix = "jetty.graph.";
         colorModuleBg = getProperty(props, prefix + "color.module.bg", colorModuleBg);
         colorEnabledBg = getProperty(props, prefix + "color.enabled.bg", colorEnabledBg);
+        colorEdgeBefore = getProperty(props, prefix + "color.edge.before", colorEdgeBefore);
+        colorEdgeAfter = getProperty(props, prefix + "color.edge.after", colorEdgeAfter);
         colorTransitiveBg = getProperty(props, prefix + "color.transitive.bg", colorTransitiveBg);
         colorCellBg = getProperty(props, prefix + "color.cell.bg", colorCellBg);
         colorHeaderBg = getProperty(props, prefix + "color.header.bg", colorHeaderBg);
@@ -73,7 +79,7 @@ private String getProperty(Props props, String key, String defVal)
     public void write(Modules modules, Path outputFile) throws IOException
     {
         try (BufferedWriter writer = Files.newBufferedWriter(outputFile, StandardCharsets.UTF_8, StandardOpenOption.CREATE_NEW, StandardOpenOption.WRITE);
-             PrintWriter out = new PrintWriter(writer);)
+             PrintWriter out = new PrintWriter(writer))
         {
             writeHeaderMessage(out, outputFile);
 
@@ -245,13 +251,13 @@ private void writeRelationships(PrintWriter out, Iterable<Module> modules, List<
                 depends = Module.normalizeModuleName(depends);
                 out.printf("    \"%s\" -> \"%s\";%n", module.getName(), depends);
             }
-            for (String before : module.getBefore())
+            for (String optional : module.getAfter())
             {
-                out.printf("    \"%s\" << \"%s\";%n", module.getName(), before);
+                out.printf("    \"%s\" -> \"%s\" [ color=\"%s\" ];%n", module.getName(), optional, colorEdgeAfter);
             }
-            for (String after : module.getAfter())
+            for (String before : module.getBefore())
             {
-                out.printf("    \"%s\" >> \"%s\";%n", module.getName(), after);
+                out.printf("    \"%s\" -> \"%s\" [ color=\"%s\" ];%n", before, module.getName(), colorEdgeBefore);
             }
         }
     }

diff --git a/jetty-core/jetty-start/src/test/java/org/eclipse/jetty/start/ModuleGraphWriterTest.java b/jetty-core/jetty-start/src/test/java/org/eclipse/jetty/start/ModuleGraphWriterTest.java
@@ -13,8 +13,12 @@
 
 package org.eclipse.jetty.start;
 
+import java.io.BufferedReader;
 import java.io.IOException;
+import java.io.InputStreamReader;
+import java.nio.charset.StandardCharsets;
 import java.nio.file.Path;
+import java.time.Duration;
 
 import org.eclipse.jetty.start.config.CommandLineConfigSource;
 import org.eclipse.jetty.start.config.ConfigSources;
@@ -28,6 +32,8 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.is;
+import static org.junit.jupiter.api.Assertions.assertTimeout;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 @ExtendWith(WorkDirExtension.class)
 public class ModuleGraphWriterTest
@@ -58,11 +64,51 @@ public void testGenerateNothingEnabled() throws IOException
         Modules modules = new Modules(basehome, args);
         modules.registerAll();
 
-        Path outputFile = basehome.getBasePath("graph.dot");
+        Path dotFile = basehome.getBasePath("graph.dot");
 
         ModuleGraphWriter writer = new ModuleGraphWriter();
-        writer.write(modules, outputFile);
+        writer.write(modules, dotFile);
 
-        assertThat("Output File Exists", FS.exists(outputFile), is(true));
+        assertThat("Output File Exists", FS.exists(dotFile), is(true));
+
+        assertTimeout(Duration.ofSeconds(3), () ->
+        {
+            if (execDotCmd("dot", "-V"))
+            {
+                Path outputPng = testdir.getPath().resolve("output.png");
+                assertTrue(execDotCmd("dot", "-Tpng", "-o" + outputPng, dotFile.toString()));
+
+                assertThat("PNG File does not exist", FS.exists(outputPng));
+            }
+        });
+    }
+
+    private boolean execDotCmd(String... args)
+    {
+        try
+        {
+            Process p = Runtime.getRuntime().exec(args);
+
+            try (BufferedReader bri = new BufferedReader(new InputStreamReader(p.getInputStream(), StandardCharsets.UTF_8));
+                 BufferedReader bre = new BufferedReader(new InputStreamReader(p.getErrorStream(), StandardCharsets.UTF_8)))
+            {
+                String line;
+                while ((line = bri.readLine()) != null)
+                {
+                    System.out.printf("[STDIN] %s%n", line);
+                }
+                while ((line = bre.readLine()) != null)
+                {
+                    System.out.printf("[STDERR] %s%n", line);
+                }
+            }
+            p.waitFor();
+            return true;
+        }
+        catch (IOException | InterruptedException e)
+        {
+            e.printStackTrace();
+            return false;
+        }
     }
 }