michael-o commented on a change in pull request #60:
URL: https://github.com/apache/maven-shared-utils/pull/60#discussion_r461593712
##########
File path:
src/test/java/org/apache/maven/shared/utils/cli/CommandLineUtilsTest.java
##########
@@ -168,4 +169,25 @@ private void assertCmdLineArgs( final String[] expected,
final String cmdLine )
assertEquals( expected.length, actual.length );
assertEquals( Arrays.asList( expected ), Arrays.asList( actual ) );
}
+
+ @Test
+ public void testChineseEncodingIssue()
+ throws Exception
+ {
+ Commandline commandline = new Commandline( "ping www.baidu.com" );
Review comment:
No, we cannot have tests which require outbound access. Espcially ICMP.
Please change test.
##########
File path: src/main/java/org/apache/maven/shared/utils/cli/CommandLineUtils.java
##########
@@ -280,11 +280,11 @@ public Integer call()
inputFeeder.start();
}
- outputPumper = new StreamPumper( p.getInputStream(),
systemOut );
+ outputPumper = new StreamPumper( p.getInputStream(),
systemOut , streamCharset );
Review comment:
Space before comma is not required.
##########
File path:
src/test/java/org/apache/maven/shared/utils/cli/CommandLineUtilsTest.java
##########
@@ -168,4 +169,25 @@ private void assertCmdLineArgs( final String[] expected,
final String cmdLine )
assertEquals( expected.length, actual.length );
assertEquals( Arrays.asList( expected ), Arrays.asList( actual ) );
}
+
+ @Test
+ public void testChineseEncodingIssue()
Review comment:
This test is completely pointless:
1. The `echo` will use the encoding supplied the entire system. There is no
guarantee that GBK is the system encoding.
2. You never verify the output of th command to be what you expect.
What you require is an application that wroduces GBK *bytes* , those are
read by Java with GBK into a `String` then you need to compared this value.
##########
File path:
src/test/java/org/apache/maven/shared/utils/cli/CommandLineUtilsTest.java
##########
@@ -168,4 +169,25 @@ private void assertCmdLineArgs( final String[] expected,
final String cmdLine )
assertEquals( expected.length, actual.length );
assertEquals( Arrays.asList( expected ), Arrays.asList( actual ) );
}
+
+ @Test
+ public void testChineseEncodingIssue()
Review comment:
This test is completely pointless:
1. The `echo` will use the encoding supplied by the entire system. There is
no guarantee that GBK is the system encoding.
2. You never verify the output of th command to be what you expect.
What you need is an application that produces GBK *bytes* , those are read
by Java with GBK into a `String` then you need to compare this value.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
