> On June 21, 2015, 5:49 a.m., Guozhang Wang wrote: > > I think the main reason is that util.Random.nextString() may include other > > non-ascii chars and hence its toString may not be well-defined: > > > > http://alvinalexander.com/scala/creating-random-strings-in-scala > > > > So I think bottom-line is that we should not use util.Random.nextString to > > generate random strings. There are a couple of other places where it it > > used and I suggest we remove them as well.
Since Java uses utf-16 internally, nextString is virtually guaranteed to be non-ascii. But the problem was actually the additional space which was being trimmed. Nevertheless, we might want to avoid using non-ascii characters in assertions unles we can find a reasonable way to display them in test results. The other usages seem legitimate though. One of them explicitly expects non-ascii strings (ApiUtilsTest.testShortStringNonASCII), and the others just use them as arbitrary data of a certain length (OffsetCommitTest.testLargeMetadataPayload). - Jason ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35655/#review88690 ----------------------------------------------------------- On June 19, 2015, 4:48 p.m., Jason Gustafson wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35655/ > ----------------------------------------------------------- > > (Updated June 19, 2015, 4:48 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-2271 > https://issues.apache.org/jira/browse/KAFKA-2271 > > > Repository: kafka > > > Description > ------- > > KAFKA-2271; fix minor test bugs > > > Diffs > ----- > > core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala > 98a5b042a710d3c1064b0379db1d152efc9eabee > > Diff: https://reviews.apache.org/r/35655/diff/ > > > Testing > ------- > > > Thanks, > > Jason Gustafson > >
