> On June 22, 2015, 11:43 p.m., Ewen Cheslack-Postava wrote: > > LGTM. There were 9 question marks when 10 characters were requested, so the > > problem was probably just that a whitespace character at the start or end > > would get trimmed during AbstractConfig's parsing. > > Jason Gustafson wrote: > That's what I thought as well, but I was puzzled that I couldn't > reproduce it. In fact, it looks like the issue was fixed with KAFKA-2249, > which preserves the original properties that were used to construct the > config. In that case, however, the assertion basically becomes a tautology, > so perhaps we should just remove the test case?
That makes sense. I agree that this test doesn't seem all that useful anymore. I think all the old config classes had tests since they were all written manually. Since the configs are now defined more declaratively via AbstractConfig, these types of tests seem a lot less relevant. It doesn't look like we've generated tests for any other uses of AbstractConfig. Even other tests in that same class are just sort of redundant, e.g. testFromPropsInvalid. It is, I suppose checking that the type is set correctly in the ConfigDef, but mostly it's just retesting the common parsing functionality repeatedly. - Ewen ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35655/#review88877 ----------------------------------------------------------- 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 > >
