From b606e025f10daed18b90b45ac00cd0c82e818581 Mon Sep 17 00:00:00 2001 From: Wei-Chiu Chuang Date: Fri, 9 Dec 2016 15:22:21 -0800 Subject: [PATCH] HADOOP-13824. FsShell can suppress the real error if no error message is present. Contributed by John Zhuge. --- .../java/org/apache/hadoop/fs/FsShell.java | 7 +- .../org/apache/hadoop/fs/TestFsShell.java | 67 ++++++++++------- .../apache/hadoop/test/GenericTestUtils.java | 72 ++++++++++++++++++- 3 files changed, 117 insertions(+), 29 deletions(-) diff --git a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShell.java b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShell.java index 1de9374b80..59d15c2716 100644 --- a/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShell.java +++ b/hadoop-common-project/hadoop-common/src/main/java/org/apache/hadoop/fs/FsShell.java @@ -328,7 +328,12 @@ public int run(String argv[]) throws Exception { scope.close(); } } catch (IllegalArgumentException e) { - displayError(cmd, e.getLocalizedMessage()); + if (e.getMessage() == null) { + displayError(cmd, "Null exception message"); + e.printStackTrace(System.err); + } else { + displayError(cmd, e.getLocalizedMessage()); + } printUsage(System.err); if (instance != null) { printInstanceUsage(System.err, instance); diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShell.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShell.java index 376f8a6f67..162a9421d3 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShell.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFsShell.java @@ -17,18 +17,19 @@ */ package org.apache.hadoop.fs; -import java.io.ByteArrayOutputStream; -import java.io.PrintStream; - import junit.framework.AssertionFailedError; import org.apache.hadoop.conf.Configuration; -import org.apache.hadoop.io.IOUtils; +import org.apache.hadoop.fs.shell.Command; +import org.apache.hadoop.fs.shell.CommandFactory; +import org.apache.hadoop.test.GenericTestUtils; import org.apache.hadoop.tracing.SetSpanReceiver; import org.apache.hadoop.util.ToolRunner; import org.apache.htrace.core.AlwaysSampler; import org.apache.htrace.core.Tracer; +import org.hamcrest.core.StringContains; import org.junit.Assert; import org.junit.Test; +import org.mockito.Mockito; public class TestFsShell { @@ -73,30 +74,42 @@ public void testTracing() throws Throwable { @Test public void testDFSWithInvalidCommmand() throws Throwable { - Configuration conf = new Configuration(); - FsShell shell = new FsShell(conf); - String[] args = new String[1]; - args[0] = "dfs -mkdirs"; - final ByteArrayOutputStream bytes = new ByteArrayOutputStream(); - final PrintStream out = new PrintStream(bytes); - final PrintStream oldErr = System.err; - try { - System.setErr(out); - ToolRunner.run(shell, args); - String errorValue=new String(bytes.toString()); - Assert - .assertTrue( - "FSShell dfs command did not print the error " + - "message when invalid command is passed", - errorValue.contains("-mkdirs: Unknown command")); - Assert - .assertTrue( - "FSShell dfs command did not print help " + + FsShell shell = new FsShell(new Configuration()); + try (GenericTestUtils.SystemErrCapturer capture = + new GenericTestUtils.SystemErrCapturer()) { + ToolRunner.run(shell, new String[]{"dfs -mkdirs"}); + Assert.assertThat("FSShell dfs command did not print the error " + "message when invalid command is passed", - errorValue.contains("Usage: hadoop fs [generic options]")); - } finally { - IOUtils.closeStream(out); - System.setErr(oldErr); + capture.getOutput(), StringContains.containsString( + "-mkdirs: Unknown command")); + Assert.assertThat("FSShell dfs command did not print help " + + "message when invalid command is passed", + capture.getOutput(), StringContains.containsString( + "Usage: hadoop fs [generic options]")); + } + } + + @Test + public void testExceptionNullMessage() throws Exception { + final String cmdName = "-cmdExNullMsg"; + final Command cmd = Mockito.mock(Command.class); + Mockito.when(cmd.run(Mockito.anyVararg())).thenThrow( + new IllegalArgumentException()); + Mockito.when(cmd.getUsage()).thenReturn(cmdName); + + final CommandFactory cmdFactory = Mockito.mock(CommandFactory.class); + final String[] names = {cmdName}; + Mockito.when(cmdFactory.getNames()).thenReturn(names); + Mockito.when(cmdFactory.getInstance(cmdName)).thenReturn(cmd); + + FsShell shell = new FsShell(new Configuration()); + shell.commandFactory = cmdFactory; + try (GenericTestUtils.SystemErrCapturer capture = + new GenericTestUtils.SystemErrCapturer()) { + ToolRunner.run(shell, new String[]{cmdName}); + Assert.assertThat(capture.getOutput(), + StringContains.containsString(cmdName + + ": Null exception message")); } } } diff --git a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/GenericTestUtils.java b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/GenericTestUtils.java index 0b73cf52a8..96ba123de1 100644 --- a/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/GenericTestUtils.java +++ b/hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/test/GenericTestUtils.java @@ -18,10 +18,13 @@ package org.apache.hadoop.test; import java.io.BufferedReader; +import java.io.ByteArrayOutputStream; import java.io.File; import java.io.FileInputStream; import java.io.IOException; import java.io.InputStreamReader; +import java.io.OutputStream; +import java.io.PrintStream; import java.io.StringWriter; import java.lang.management.ManagementFactory; import java.lang.management.ThreadInfo; @@ -272,7 +275,74 @@ public static void waitFor(Supplier check, "Thread diagnostics:\n" + TimedOutTestsListener.buildThreadDiagnosticString()); } - + + /** + * Prints output to one {@link PrintStream} while copying to the other. + *

+ * Closing the main {@link PrintStream} will NOT close the other. + */ + public static class TeePrintStream extends PrintStream { + private final PrintStream other; + + public TeePrintStream(OutputStream main, PrintStream other) { + super(main); + this.other = other; + } + + @Override + public void flush() { + super.flush(); + other.flush(); + } + + @Override + public void write(byte[] buf, int off, int len) { + super.write(buf, off, len); + other.write(buf, off, len); + } + } + + /** + * Capture output printed to {@link System#err}. + *

+ * Usage: + * 

+   *   try (SystemErrCapturer capture = new SystemErrCapturer()) {
+   *     ...
+   *     // Call capture.getOutput() to get the output string
+   *   }
+   *
+ * + * TODO: Add lambda support once Java 8 is common. + * 
+   *   SystemErrCapturer.withCapture(capture -> {
+   *     ...
+   *   })
+   *
+ */ + public static class SystemErrCapturer implements AutoCloseable { + final private ByteArrayOutputStream bytes; + final private PrintStream bytesPrintStream; + final private PrintStream oldErr; + + public SystemErrCapturer() { + bytes = new ByteArrayOutputStream(); + bytesPrintStream = new PrintStream(bytes); + oldErr = System.err; + System.setErr(new TeePrintStream(oldErr, bytesPrintStream)); + } + + public String getOutput() { + return bytes.toString(); + } + + @Override + public void close() throws Exception { + IOUtils.closeQuietly(bytesPrintStream); + System.setErr(oldErr); + } + } + public static class LogCapturer { private StringWriter sw = new StringWriter(); private WriterAppender appender;