For what it is worth I run a full mvn clean install from the command line before most commits. Yes it takes time but I think it is necessary.
Sent from my iPhone > On May 25, 2014, at 6:24 AM, Gary Gregory <[email protected]> wrote: > > The type converters still seems like a great idea. I've never liked having > all of this boilerplate conversion code all over that place, even if most of > it was refactored into calls to utils. It's too easy to make a mistake. > > WRT default values, then yes, we need to carefully consider what to do here, > but since we are the primary customer, we can do whatever we want especially > for the tests. > > Finger waving? ;-) sometimes a big pile of changes is safe, sometimes one > character change is not... :-( What happened to running Log4J on a CI server? > This would not solve this process issue but it would feedback an error to the > ML and the committer ASAP. > > Gary > > >> On Sun, May 25, 2014 at 8:11 AM, Remko Popma <[email protected]> wrote: >> I'm seeing a compile error in FlumeAppender (after the change to >> Rfc5424Layout.createLayout). I fixed that, but now there are >> few broken JUnit tests for the Flume appender: >> >> Tests in error: >> FlumeAppenderTest.testBatch:237 ≫ IllegalArgument No structured id name >> was su... >> FlumeAppenderTest.testStructured:163 ≫ IllegalArgument No structured id >> name w... >> FlumeAppenderTest.testLog4jAvroAppender:134 ≫ IllegalArgument No >> structured id... >> FlumeAppenderTest.testNotConnected:301 ≫ IllegalArgument No structured id >> name... >> FlumeAppenderTest.testMultiple:203 ≫ IllegalArgument No structured id name >> was... >> FlumeAppenderTest.testReconnect:349 ≫ IllegalArgument No structured id >> name wa... >> FlumeAppenderTest.testConnectionRefused:271 ≫ IllegalArgument No >> structured id... >> >> detail: >> >> testBatch(org.apache.logging.log4j.flume.appender.FlumeAppenderTest) Time >> elapsed: 3.305 sec <<< ERROR! >> java.lang.IllegalArgumentException: No structured id name was supplied >> at >> org.apache.logging.log4j.message.StructuredDataId.<init>(StructuredDataId.java:92) >> at >> org.apache.logging.log4j.core.layout.Rfc5424Layout.<init>(Rfc5424Layout.java:136) >> at >> org.apache.logging.log4j.core.layout.Rfc5424Layout.createLayout(Rfc5424Layout.java:652) >> at >> org.apache.logging.log4j.flume.appender.FlumeAppender.createAppender(FlumeAppender.java:223) >> at >> org.apache.logging.log4j.flume.appender.FlumeAppenderTest.testBatch(FlumeAppenderTest.java:237) >> >> >> First, the mandatory finger wagging: please please run the tests before >> committing, etc, etc. >> >> Second, I'm looking at how to fix these tests and now I'm not sure whether >> the default annotation was a good idea: the thing is that the JUnit tests >> use the factory methods directly but there is no PluginManager involved, so >> nobody supplies the default value any more... >> >> That is why the JUnit tests above fail: FlumeAppender.createAppender() >> passes a null argument to Rfc5424Layout.createLayout(), which is no longer >> converted to the default value, which causes issues downstream. >> >> This raises the question: how do we deal with this? >> >> Should we change the FlumeAppender.createAppender() to provide a non-null >> value? >> That is possible of course, but then we only test the cases where a value >> was supplied. Would we not be missing test cases this way? >> >> UPDATE: >> I've tried providing a non-null value but the tests still fail. >> >> I want to work on other things now so I'm giving up on this. Matt, could you >> take a look? >> >> Error detail: >> testBatch(org.apache.logging.log4j.flume.appender.FlumeAppenderTest) Time >> elapsed: 3.315 sec <<< ERROR! >> org.apache.logging.log4j.core.appender.AppenderLoggingException: An >> exception occurred processing Appender avro >> at >> org.apache.logging.log4j.core.net.Priority.getPriority(Priority.java:46) >> at >> org.apache.logging.log4j.core.layout.Rfc5424Layout.appendPriority(Rfc5424Layout.java:283) >> at >> org.apache.logging.log4j.core.layout.Rfc5424Layout.toSerializable(Rfc5424Layout.java:262) >> at >> org.apache.logging.log4j.core.layout.Rfc5424Layout.toSerializable(Rfc5424Layout.java:63) >> at >> org.apache.logging.log4j.core.layout.AbstractStringLayout.toByteArray(AbstractStringLayout.java:46) >> at >> org.apache.logging.log4j.flume.appender.FlumeAppender.append(FlumeAppender.java:102) >> at >> org.apache.logging.log4j.core.config.AppenderControl.callAppender(AppenderControl.java:97) >> at >> org.apache.logging.log4j.core.config.LoggerConfig.callAppenders(LoggerConfig.java:426) >> at >> org.apache.logging.log4j.core.config.LoggerConfig.log(LoggerConfig.java:405) >> at >> org.apache.logging.log4j.core.config.LoggerConfig.log(LoggerConfig.java:365) >> at org.apache.logging.log4j.core.Logger.logMessage(Logger.java:103) >> at >> org.apache.logging.log4j.spi.AbstractLoggerProvider.logMessage(AbstractLoggerProvider.java:1336) >> at >> org.apache.logging.log4j.spi.AbstractLoggerProvider.logIfEnabled(AbstractLoggerProvider.java:1325) >> at >> org.apache.logging.log4j.spi.AbstractLoggerProvider.info(AbstractLoggerProvider.java:907) >> at >> org.apache.logging.log4j.flume.appender.FlumeAppenderTest.testBatch(FlumeAppenderTest.java:248) > > > > -- > E-Mail: [email protected] | [email protected] > Java Persistence with Hibernate, Second Edition > JUnit in Action, Second Edition > Spring Batch in Action > Blog: http://garygregory.wordpress.com > Home: http://garygregory.com/ > Tweet! http://twitter.com/GaryGregory
