Remote console note test coverage & fixes

src/main/java/hudson/plugins/ansicolor/AnsiAttributeElement.java

@@ -64,4 +64,34 @@ public int hashCode() {
         result = 31 * result + attributes.hashCode();
         return result;
     }
+
+    public static AnsiAttributeElement bold() {
+        return new AnsiAttributeElement(AnsiAttributeElement.AnsiAttrType.BOLD, "b", "");
+    }
+
+    public static AnsiAttributeElement italic() {
+        return new AnsiAttributeElement(AnsiAttrType.ITALIC, "i", "");
+    }
+
+    public static AnsiAttributeElement underline() {
+        return new AnsiAttributeElement(AnsiAttrType.UNDERLINE, "u", "");
+    }
+
+    public static AnsiAttributeElement underlineDouble() {
+        return new AnsiAttributeElement(AnsiAttrType.UNDERLINE, "span", "style=\"border-bottom: 3px double;\"");
+    }
+
+    public static AnsiAttributeElement strikeout() {
+        return new AnsiAttributeElement(AnsiAttrType.STRIKEOUT, "span", "style=\"text-decoration: line-through;\"");
+    }
+
+    public static AnsiAttributeElement framed() {
+        return new AnsiAttributeElement(AnsiAttrType.FRAMED, "span", "style=\"border: 1px solid;\"");
+    }
+
+    public static AnsiAttributeElement overline() {
+        return new AnsiAttributeElement(AnsiAttrType.OVERLINE, "span", "style=\"text-decoration: overline;\"");
+        // return new AnsiAttributeElement(AnsiAttrType.OVERLINE, "span", "style=\"border-top: 1px solid;\""); // alternate approach
+    }
+
 }

src/main/java/hudson/plugins/ansicolor/AnsiColorConsoleLogFilter.java

@@ -1,7 +1,6 @@
 package hudson.plugins.ansicolor;
 
 import hudson.console.ConsoleLogFilter;
-import hudson.console.LineTransformationOutputStream;
 import hudson.model.AbstractBuild;
 import java.io.ByteArrayOutputStream;
 
@@ -33,7 +32,15 @@ public AnsiColorConsoleLogFilter(AnsiColorMap colorMap) {
         // some cases of AnsiHtmlOutputStream.setForegroundColor:
         for (AnsiColorMap.Color color : AnsiColorMap.Color.values()) {
             pregenerateNote(new AnsiAttributeElement(AnsiAttributeElement.AnsiAttrType.FG, "span", "style=\"color: " + colorMap.getNormal(color.ordinal()) + ";\""));
+            pregenerateNote(new AnsiAttributeElement(AnsiAttributeElement.AnsiAttrType.FG, "span", "style=\"color: " + colorMap.getBright(color.ordinal()) + ";\""));
         }
+        pregenerateNote(AnsiAttributeElement.bold());
+        pregenerateNote(AnsiAttributeElement.italic());
+        pregenerateNote(AnsiAttributeElement.underline());
+        pregenerateNote(AnsiAttributeElement.underlineDouble());
+        pregenerateNote(AnsiAttributeElement.strikeout());
+        pregenerateNote(AnsiAttributeElement.framed());
+        pregenerateNote(AnsiAttributeElement.overline());
         // TODO other cases, and other methods
         LOG.log(Level.FINE, "Notes pregenerated for {0}", notes.keySet());
     }
@@ -68,36 +75,21 @@ public OutputStream decorateLogger(AbstractBuild build, final OutputStream logge
             return null;
         }
 
-        return new LineTransformationOutputStream() {
-            AnsiHtmlOutputStream ansi = new AnsiHtmlOutputStream(logger, colorMap, new AnsiAttributeElement.Emitter() {
-                @Override
-                public void emitHtml(String html) {
-                    try {
-                        byte[] pregenerated = notes.get(html);
-                        if (pregenerated != null) {
-                            logger.write(pregenerated);
-                        } else {
-                            new SimpleHtmlNote(html).encodeTo(logger);
-                        }
-                    } catch (IOException e) {
-                        LOG.log(Level.WARNING, "Failed to add HTML markup '" + html + "'", e);
+        return new AnsiHtmlOutputStream(logger, colorMap, new AnsiAttributeElement.Emitter() {
+            @Override
+            public void emitHtml(String html) {
+                try {
+                    byte[] pregenerated = notes.get(html);
+                    if (pregenerated != null) {
+                        logger.write(pregenerated);
+                    } else {
+                        // TODO decline to use pregenerated form of end tag if start tag could not be pregenerated
+                        new SimpleHtmlNote(html).encodeTo(logger);
                     }
+                } catch (IOException e) {
+                    LOG.log(Level.WARNING, "Failed to add HTML markup '" + html + "'", e);
                 }
-            });
-
-            @Override
-            protected void eol(byte[] b, int len) throws IOException {
-                ansi.write(b, 0, len);
-                ansi.flush();
-                logger.flush();
-            }
-
-            @Override
-            public void close() throws IOException {
-                ansi.close();
-                logger.close();
-                super.close();
             }
-        };
+        });
     }
 }

src/main/java/hudson/plugins/ansicolor/AnsiHtmlOutputStream.java

@@ -391,28 +391,28 @@ else switch (attribute) {
             break;
         case ATTRIBUTE_INTENSITY_BOLD:
             closeTagOfType(AnsiAttrType.BOLD);
-            openTag(new AnsiAttributeElement(AnsiAttrType.BOLD, "b", ""));
+            openTag(AnsiAttributeElement.bold());
             break;
         case ATTRIBUTE_INTENSITY_NORMAL:
             closeTagOfType(AnsiAttrType.BOLD);
             break;
         case ATTRIBUTE_ITALIC:
             closeTagOfType(AnsiAttrType.ITALIC);
-            openTag(new AnsiAttributeElement(AnsiAttrType.ITALIC, "i", ""));
+            openTag(AnsiAttributeElement.italic());
             break;
         case ATTRIBUTE_ITALIC_OFF:
             closeTagOfType(AnsiAttrType.ITALIC);
             break;
         case ATTRIBUTE_UNDERLINE:
             closeTagOfType(AnsiAttrType.UNDERLINE);
-            openTag(new AnsiAttributeElement(AnsiAttrType.UNDERLINE, "u", ""));
+            openTag(AnsiAttributeElement.underline());
             break;
         case ATTRIBUTE_UNDERLINE_DOUBLE:
             // Double underlining is handled entirely different from single underlining, by using a CSS border
             // instead of a u-element, but it's still of the same attribute type and previously opened elements of
             // either type are closed accordingly.
             closeTagOfType(AnsiAttrType.UNDERLINE);
-            openTag(new AnsiAttributeElement(AnsiAttrType.UNDERLINE, "span", "style=\"border-bottom: 3px double;\""));
+            openTag(AnsiAttributeElement.underlineDouble());
             break;
         case ATTRIBUTE_UNDERLINE_OFF:
             closeTagOfType(AnsiAttrType.UNDERLINE);
@@ -447,23 +447,22 @@ else switch (attribute) {
             // alternatives are <del> <s> (both tested and successfully rendered in firefox 51.0.1)
             // but I finally decide for "text-decoration: line-through"
             closeTagOfType(AnsiAttrType.STRIKEOUT);
-            openTag(new AnsiAttributeElement(AnsiAttrType.STRIKEOUT, "span", "style=\"text-decoration: line-through;\""));
+            openTag(AnsiAttributeElement.strikeout());
             // openTag(new AnsiAttributeElement(AnsiAttrType.STRIKEOUT, "s", "")); // alternate approach
             break;
         case ATTRIBUTE_STRIKEOUT_OFF:
             closeTagOfType(AnsiAttrType.STRIKEOUT);
             break;
         case ATTRIBUTE_FRAMED:
             closeTagOfType(AnsiAttrType.FRAMED);
-            openTag(new AnsiAttributeElement(AnsiAttrType.FRAMED, "span", "style=\"border: 1px solid;\""));
+            openTag(AnsiAttributeElement.framed());
             break;
         case ATTRIBUTE_FRAMED_OFF:
             closeTagOfType(AnsiAttrType.FRAMED);
             break;
         case ATTRIBUTE_OVERLINE:
             closeTagOfType(AnsiAttrType.OVERLINE);
-            openTag(new AnsiAttributeElement(AnsiAttrType.OVERLINE, "span", "style=\"text-decoration: overline;\""));
-            //openTag(new AnsiAttributeElement(AnsiAttrType.OVERLINE, "span", "style=\"border-top: 1px solid;\"")); // alternate approach
+            openTag(AnsiAttributeElement.overline());
             break;
         case ATTRIBUTE_OVERLINE_OFF:
             closeTagOfType(AnsiAttrType.OVERLINE);

src/test/java/hudson/plugins/ansicolor/AnsiColorConsoleLogFilterTest.java

@@ -0,0 +1,178 @@
+/*
+ * The MIT License
+ *
+ * Copyright 2018 CloudBees, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a copy
+ * of this software and associated documentation files (the "Software"), to deal
+ * in the Software without restriction, including without limitation the rights
+ * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
+ * copies of the Software, and to permit persons to whom the Software is
+ * furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
+ * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
+ * THE SOFTWARE.
+ */
+
+package hudson.plugins.ansicolor;
+
+import hudson.console.ConsoleAnnotationOutputStream;
+import hudson.model.AbstractBuild;
+import hudson.model.TaskListener;
+import hudson.slaves.DumbSlave;
+import hudson.util.StreamTaskListener;
+import java.io.IOException;
+import java.io.OutputStream;
+import java.io.PrintStream;
+import java.io.StringWriter;
+import java.nio.charset.StandardCharsets;
+import java.util.logging.Level;
+import jenkins.security.MasterToSlaveCallable;
+import org.jenkinsci.plugins.workflow.log.ConsoleAnnotators;
+import org.junit.AssumptionViolatedException; // Ignore seems to be ignored in this context
+import org.junit.ClassRule;
+import org.junit.BeforeClass;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.LoggerRule;
+
+/**
+ * Checks which kinds of console notes are successfully pregenerated for use in a remoted filter.
+ */
+@Issue("JENKINS-54133")
+public class AnsiColorConsoleLogFilterTest extends AnsiHtmlOutputStreamTest {
+
+    @ClassRule
+    public static LoggerRule logging = new LoggerRule().record(AnsiColorConsoleLogFilter.class, Level.FINE);
+
+    @ClassRule
+    public static JenkinsRule r = new JenkinsRule();
+
+    private static DumbSlave s;
+
+    @BeforeClass
+    public static void createSlave() throws Exception {
+        s = r.createOnlineSlave();
+    }
+
+    @Override
+    protected String annotate(String text, AnsiColorMap colorMap) throws IOException {
+        StringWriter sw = new StringWriter();
+        try (OutputStream caos = new ConsoleAnnotationOutputStream<Void>(sw, ConsoleAnnotators.createAnnotator(null), null, StandardCharsets.UTF_8);
+             StreamTaskListener listener = new StreamTaskListener(caos)) {
+            s.getChannel().call(new AnnotateCallable(text, listener, new AnsiColorConsoleLogFilter(colorMap)));
+        } catch (IOException x) {
+            throw x;
+        } catch (Exception x) {
+            throw new IOException(x);
+        }
+        return sw.toString();
+    }
+
+    private static final class AnnotateCallable extends MasterToSlaveCallable<Void, Exception> {
+
+        private final String text;
+        private final TaskListener listener;
+        private final AnsiColorConsoleLogFilter filter;
+
+        AnnotateCallable(String text, TaskListener listener, AnsiColorConsoleLogFilter filter) {
+            this.text = text;
+            this.listener = listener;
+            this.filter = filter;
+        }
+
+        @Override
+        public Void call() throws Exception {
+            try (OutputStream decorated = filter.decorateLogger((AbstractBuild) null, listener.getLogger());
+                 PrintStream ps = new PrintStream(decorated)) {
+                ps.print(text);
+            }
+            return null;
+        }
+
+    }
+
+    @Override
+    public void testEmbeddedConsoleNote() throws IOException {
+        throw new AssumptionViolatedException("seems irrelevant");
+    }
+
+    @Override
+    public void testNegative() throws IOException {
+        throw new AssumptionViolatedException("TODO not implemented");
+    }
+
+    @Override
+    public void testGreenOnWhite() throws IOException {
+        throw new AssumptionViolatedException("TODO missing background-color");
+    }
+
+    @Override
+    public void testGreenOnWhiteCSS() throws IOException {
+        throw new AssumptionViolatedException("TODO missing background-color");
+    }
+
+    @Override
+    public void testGreenOnWhiteXTerm() throws IOException {
+        throw new AssumptionViolatedException("TODO missing background-color");
+    }
+
+    @Override
+    public void testResetForegroundColor() throws IOException {
+        throw new AssumptionViolatedException("TODO missing bold");
+    }
+
+    @Override
+    public void testForegroundColor256() throws IOException {
+        throw new AssumptionViolatedException("other than the standard colors, which could be split into a separate test, seems unimplementable");
+    }
+
+    @Override
+    public void testForegroundColorRgb() throws IOException {
+        throw new AssumptionViolatedException("probably unimplementable");
+    }
+
+    @Override
+    public void testResetBackgroundColor() throws IOException {
+        throw new AssumptionViolatedException("TODO not implemented");
+    }
+
+    @Override
+    public void testBackgroundColorHighIntensity() throws IOException {
+        throw new AssumptionViolatedException("TODO not implemented");
+    }
+
+    @Override
+    public void testBackgroundColor256() throws IOException {
+        throw new AssumptionViolatedException("other than the standard colors, which could be split into a separate test, seems unimplementable");
+    }
+
+    @Override
+    public void testBackgroundColorRgb() throws IOException {
+        throw new AssumptionViolatedException("probably unimplementable");
+    }
+
+    @Override
+    public void testDefaultColors() throws IOException {
+        throw new AssumptionViolatedException("TODO missing background-color");
+    }
+
+    @Override
+    public void testConsoleNote() throws IOException {
+        throw new AssumptionViolatedException("seems irrelevant");
+    }
+
+    @Override
+    public void testOverlapping() throws IOException {
+        throw new AssumptionViolatedException("TODO missing some things");
+    }
+
+}

src/test/java/hudson/plugins/ansicolor/AnsiHtmlOutputStreamTest.java

@@ -610,7 +610,7 @@ private void assertThatAnnotateIs(AnsiColorMap colorMap, String ansi, String htm
         assertThat(annotate(ansi, colorMap), is(html));
     }
 
-    private String annotate(String text, AnsiColorMap colorMap) throws IOException {
+    protected String annotate(String text, AnsiColorMap colorMap) throws IOException {
         final ByteArrayOutputStream bos = new ByteArrayOutputStream();
         AnsiHtmlOutputStream ansi = new AnsiHtmlOutputStream(bos, colorMap, new AnsiAttributeElement.Emitter() {
             public void emitHtml(String html) {