-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/49830/#review141357
-----------------------------------------------------------


Ship it!




- Hari Shreedharan


On July 8, 2016, 10:04 p.m., Mike Percy wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/49830/
> -----------------------------------------------------------
> 
> (Updated July 8, 2016, 10:04 p.m.)
> 
> 
> Review request for Flume and Hari Shreedharan.
> 
> 
> Bugs: FLUME-2941
>     https://issues.apache.org/jira/browse/FLUME-2941
> 
> 
> Repository: flume-git
> 
> 
> Description
> -------
> 
> This patch makes the Flume test code conform to the Google style guidelines.
> 
> This patch also makes all future style violations fatal to the build.
> 
> This patch is whitespace-only from a code perspective. After stripping
> line numbers, the generated test bytecode before and after these changes
> is identical.
> 
> 
> Diffs
> -----
> 
>   flume-checkstyle/pom.xml 31db3c0 
>   flume-checkstyle/src/main/resources/flume/checkstyle-suppressions.xml 
> 49c8834 
>   flume-checkstyle/src/main/resources/flume/checkstyle.xml e8913f0 
>   
> flume-ng-auth/src/test/java/org/apache/flume/auth/TestFlumeAuthenticator.java 
> 5a8860d 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/CountingSinkRunner.java
>  0733dc4 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/CountingSourceRunner.java
>  b6abc35 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestCheckpoint.java
>  c1de12e 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestEventQueueBackingStoreFactory.java
>  52c706d 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestEventUtils.java
>  c72e3f2 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannel.java
>  bb22e26 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannelFormatRegression.java
>  c95122b 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannelRestart.java
>  d5fe6fb 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFileChannelRollback.java
>  23fc64b 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestFlumeEventQueue.java
>  1adb21a 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestIntegration.java
>  2fbe116 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLog.java
>  b1f59cd 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestLogFile.java
>  976a112 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestTransactionEventRecordV2.java
>  2356d90 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestTransactionEventRecordV3.java
>  eb0ce04 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/TestUtils.java
>  61f38d2 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/encryption/CipherProviderTestSuite.java
>  530ccf6 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/encryption/EncryptionTestUtils.java
>  6ca3246 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/encryption/TestAESCTRNoPaddingProvider.java
>  a7c7cb2 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/encryption/TestFileChannelEncryption.java
>  d4537a8 
>   
> flume-ng-channels/flume-file-channel/src/test/java/org/apache/flume/channel/file/encryption/TestJCEFileKeyProvider.java
>  f33cada 
>   
> flume-ng-channels/flume-jdbc-channel/src/test/java/org/apache/flume/channel/jdbc/BaseJdbcChannelProviderTest.java
>  85ad7fe 
>   
> flume-ng-channels/flume-jdbc-channel/src/test/java/org/apache/flume/channel/jdbc/MockEvent.java
>  1e412c5 
>   
> flume-ng-channels/flume-jdbc-channel/src/test/java/org/apache/flume/channel/jdbc/MockEventUtils.java
>  10d8b51 
>   
> flume-ng-channels/flume-jdbc-channel/src/test/java/org/apache/flume/channel/jdbc/TestDerbySchemaHandlerQueries.java
>  362bcfa 
>   
> flume-ng-channels/flume-kafka-channel/src/test/java/org/apache/flume/channel/kafka/TestKafkaChannel.java
>  d01346a 
>   
> flume-ng-channels/flume-spillable-memory-channel/src/test/java/org/apache/flume/channel/TestSpillableMemoryChannel.java
>  1e4e819 
>   
> flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLoadBalancingLog4jAppender.java
>  267ac1d 
>   
> flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppender.java
>  1b840f3 
>   
> flume-ng-clients/flume-ng-log4jappender/src/test/java/org/apache/flume/clients/log4jappender/TestLog4jAppenderWithAvro.java
>  5899c62 
>   
> flume-ng-core/src/test/java/org/apache/flume/channel/AbstractBasicChannelSemanticsTest.java
>  59a804c 
>   
> flume-ng-core/src/test/java/org/apache/flume/channel/TestChannelProcessor.java
>  b37b823 
>   flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannel.java 
> 7851536 
>   
> flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannelConcurrency.java
>  d4ba705 
>   
> flume-ng-core/src/test/java/org/apache/flume/channel/TestMemoryChannelTransaction.java
>  b8e00d8 
>   
> flume-ng-core/src/test/java/org/apache/flume/client/avro/TestReliableSpoolingFileEventReader.java
>  4e90054 
>   
> flume-ng-core/src/test/java/org/apache/flume/formatter/output/TestBucketPath.java
>  21b972b 
>   
> flume-ng-core/src/test/java/org/apache/flume/instrumentation/TestMonitoredCounterGroup.java
>  b1f637f 
>   
> flume-ng-core/src/test/java/org/apache/flume/instrumentation/http/TestHTTPMetricsServer.java
>  eb2d02d 
>   
> flume-ng-core/src/test/java/org/apache/flume/instrumentation/kafka/KafkaSourceCounterTest.java
>  4a71265 
>   
> flume-ng-core/src/test/java/org/apache/flume/interceptor/TestRegexExtractorInterceptorMillisSerializer.java
>  ac46131 
>   
> flume-ng-core/src/test/java/org/apache/flume/interceptor/TestRegexExtractorInterceptorPassThroughSerializer.java
>  569c274 
>   
> flume-ng-core/src/test/java/org/apache/flume/interceptor/TestSearchAndReplaceInterceptor.java
>  2ab15f5 
>   
> flume-ng-core/src/test/java/org/apache/flume/serialization/SyslogAvroEventSerializer.java
>  896eced 
>   
> flume-ng-core/src/test/java/org/apache/flume/serialization/TestAvroEventDeserializer.java
>  6f9ddc2 
>   
> flume-ng-core/src/test/java/org/apache/flume/serialization/TestDurablePositionTracker.java
>  e52affb 
>   
> flume-ng-core/src/test/java/org/apache/flume/serialization/TestFlumeEventAvroEventSerializer.java
>  3860b5e 
>   
> flume-ng-core/src/test/java/org/apache/flume/serialization/TestResettableFileInputStream.java
>  631bdfe 
>   
> flume-ng-core/src/test/java/org/apache/flume/serialization/TestSyslogAvroEventSerializer.java
>  7bd342a 
>   flume-ng-core/src/test/java/org/apache/flume/sink/TestAvroSink.java 757a536 
>   
> flume-ng-core/src/test/java/org/apache/flume/sink/TestDefaultSinkFactory.java 
> 835f541 
>   
> flume-ng-core/src/test/java/org/apache/flume/sink/TestFailoverSinkProcessor.java
>  3358cf4 
>   
> flume-ng-core/src/test/java/org/apache/flume/sink/TestLoadBalancingSinkProcessor.java
>  7d95655 
>   flume-ng-core/src/test/java/org/apache/flume/sink/TestRollingFileSink.java 
> bf4ed1f 
>   flume-ng-core/src/test/java/org/apache/flume/sink/TestThriftSink.java 
> 1beec76 
>   
> flume-ng-core/src/test/java/org/apache/flume/source/TestAbstractPollableSource.java
>  d385abe 
>   flume-ng-core/src/test/java/org/apache/flume/source/TestAvroSource.java 
> c75d098 
>   flume-ng-core/src/test/java/org/apache/flume/source/TestExecSource.java 
> afa93bf 
>   
> flume-ng-core/src/test/java/org/apache/flume/source/TestMultiportSyslogTCPSource.java
>  c3dc241 
>   flume-ng-core/src/test/java/org/apache/flume/source/TestNetcatSource.java 
> e11b4b6 
>   
> flume-ng-core/src/test/java/org/apache/flume/source/TestSequenceGeneratorSource.java
>  2bbcdaf 
>   
> flume-ng-core/src/test/java/org/apache/flume/source/TestSpoolDirectorySource.java
>  47fdc7a 
>   flume-ng-core/src/test/java/org/apache/flume/source/TestStressSource.java 
> a651281 
>   flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogParser.java 
> 265157e 
>   
> flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogTcpSource.java 
> 239ba51 
>   
> flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUdpSource.java 
> 8fc80be 
>   flume-ng-core/src/test/java/org/apache/flume/source/TestSyslogUtils.java 
> 1c005ff 
>   flume-ng-core/src/test/java/org/apache/flume/source/TestThriftSource.java 
> 3d2901a 
>   
> flume-ng-core/src/test/java/org/apache/flume/source/http/FlumeHttpServletRequestWrapper.java
>  6b94b2e 
>   
> flume-ng-core/src/test/java/org/apache/flume/source/http/TestHTTPSource.java 
> c59fdd4 
>   
> flume-ng-core/src/test/java/org/apache/flume/tools/TestTimestampRoundDownUtil.java
>  cc7eac0 
>   flume-ng-core/src/test/java/org/apache/flume/tools/TestVersionInfo.java 
> b463899 
>   
> flume-ng-embedded-agent/src/test/java/org/apache/flume/agent/embedded/TestEmbeddedAgent.java
>  241e2b5 
>   
> flume-ng-embedded-agent/src/test/java/org/apache/flume/agent/embedded/TestEmbeddedAgentConfiguration.java
>  f4a9a58 
>   
> flume-ng-embedded-agent/src/test/java/org/apache/flume/agent/embedded/TestEmbeddedAgentEmbeddedSource.java
>  9d85e6e 
>   
> flume-ng-embedded-agent/src/test/java/org/apache/flume/agent/embedded/TestEmbeddedAgentState.java
>  a14a87e 
>   
> flume-ng-legacy-sources/flume-avro-source/src/test/java/org/apache/flume/source/avroLegacy/TestLegacyAvroSource.java
>  6e3eb53 
>   
> flume-ng-legacy-sources/flume-thrift-source/src/test/java/org/apache/flume/source/thriftLegacy/TestThriftLegacySource.java
>  d8a6872 
>   
> flume-ng-node/src/test/java/org/apache/flume/node/TestAbstractConfigurationProvider.java
>  15a478d 
>   
> flume-ng-node/src/test/java/org/apache/flume/node/TestAbstractZooKeeperConfigurationProvider.java
>  1ab4127 
>   flume-ng-node/src/test/java/org/apache/flume/node/TestApplication.java 
> 930f2a2 
>   
> flume-ng-node/src/test/java/org/apache/flume/node/TestPollingPropertiesFileConfigurationProvider.java
>  eed22ee 
>   
> flume-ng-node/src/test/java/org/apache/flume/node/TestPropertiesFileConfigurationProvider.java
>  84a8cfd 
>   flume-ng-node/src/test/java/org/apache/flume/source/TestNetcatSource.java 
> 91fbf63 
>   flume-ng-sdk/src/test/java/org/apache/flume/api/RpcTestUtils.java 8806860 
>   flume-ng-sdk/src/test/java/org/apache/flume/api/TestFailoverRpcClient.java 
> 64dc181 
>   
> flume-ng-sdk/src/test/java/org/apache/flume/api/TestLoadBalancingRpcClient.java
>  5d6828b 
>   flume-ng-sdk/src/test/java/org/apache/flume/api/TestNettyAvroRpcClient.java 
> cf4f415 
>   flume-ng-sdk/src/test/java/org/apache/flume/api/TestThriftRpcClient.java 
> a8baaa8 
>   flume-ng-sdk/src/test/java/org/apache/flume/api/ThriftTestingSource.java 
> 70d2c1b 
>   
> flume-ng-sinks/flume-dataset-sink/src/test/java/org/apache/flume/sink/kite/TestDatasetSink.java
>  621920d 
>   
> flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/HDFSTestSeqWriter.java
>  9c1cd09 
>   
> flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/MockDataStream.java
>  f0c6e7e 
>   
> flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/MockFileSystem.java
>  4443335 
>   
> flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/MockFsDataOutputStream.java
>  35b034e 
>   
> flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/MockHDFSWriter.java
>  ec49b97 
>   
> flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/TestBucketWriter.java
>  2581f73 
>   
> flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/TestHDFSEventSink.java
>  23862eb 
>   
> flume-ng-sinks/flume-hdfs-sink/src/test/java/org/apache/flume/sink/hdfs/TestSequenceFileSerializerFactory.java
>  6381edc 
>   
> flume-ng-sinks/flume-hive-sink/src/test/java/org/apache/flume/sink/hive/TestHiveSink.java
>  46724f2 
>   
> flume-ng-sinks/flume-hive-sink/src/test/java/org/apache/flume/sink/hive/TestHiveWriter.java
>  41bf0f6 
>   
> flume-ng-sinks/flume-hive-sink/src/test/java/org/apache/flume/sink/hive/TestUtil.java
>  107789f 
>   
> flume-ng-sinks/flume-irc-sink/src/test/java/org/apache/flume/sink/irc/TestIRCSink.java
>  e6c065e 
>   
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/AbstractElasticSearchSinkTest.java
>  f9272fa 
>   
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchIndexRequestBuilderFactory.java
>  8022111 
>   
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchLogStashEventSerializer.java
>  ab9587d 
>   
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSink.java
>  a58f344 
>   
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/TestElasticSearchSinkCreation.java
>  b5a4d2f 
>   
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/client/RoundRobinListTest.java
>  38e7399 
>   
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/client/TestElasticSearchClientFactory.java
>  4b70b65 
>   
> flume-ng-sinks/flume-ng-elasticsearch-sink/src/test/java/org/apache/flume/sink/elasticsearch/client/TestElasticSearchRestClient.java
>  1fe983a 
>   
> flume-ng-sinks/flume-ng-hbase-sink/src/test/java/org/apache/flume/sink/hbase/IncrementAsyncHBaseSerializer.java
>  b8aefe8 
>   
> flume-ng-sinks/flume-ng-hbase-sink/src/test/java/org/apache/flume/sink/hbase/TestAsyncHBaseSink.java
>  b4bbd6b 
>   
> flume-ng-sinks/flume-ng-hbase-sink/src/test/java/org/apache/flume/sink/hbase/TestHBaseSink.java
>  ab65a38 
>   
> flume-ng-sinks/flume-ng-hbase-sink/src/test/java/org/apache/flume/sink/hbase/TestRegexHbaseEventSerializer.java
>  b102b49 
>   
> flume-ng-sinks/flume-ng-kafka-sink/src/test/java/org/apache/flume/sink/kafka/TestKafkaSink.java
>  f577e98 
>   
> flume-ng-sinks/flume-ng-kafka-sink/src/test/java/org/apache/flume/sink/kafka/util/KafkaLocal.java
>  d8a45ef 
>   
> flume-ng-sinks/flume-ng-kafka-sink/src/test/java/org/apache/flume/sink/kafka/util/ZooKeeperLocal.java
>  1a5728f 
>   
> flume-ng-sinks/flume-ng-morphline-solr-sink/src/test/java/org/apache/flume/sink/solr/morphline/TestBlobDeserializer.java
>  6172c68 
>   
> flume-ng-sinks/flume-ng-morphline-solr-sink/src/test/java/org/apache/flume/sink/solr/morphline/TestMorphlineInterceptor.java
>  22cfe96 
>   
> flume-ng-sinks/flume-ng-morphline-solr-sink/src/test/java/org/apache/flume/sink/solr/morphline/TestMorphlineSolrSink.java
>  232c092 
>   
> flume-ng-sources/flume-jms-source/src/test/java/org/apache/flume/source/jms/JMSMessageConsumerTestBase.java
>  6881967 
>   
> flume-ng-sources/flume-jms-source/src/test/java/org/apache/flume/source/jms/TestDefaultJMSMessageConverter.java
>  8d413f7 
>   
> flume-ng-sources/flume-jms-source/src/test/java/org/apache/flume/source/jms/TestIntegrationActiveMQ.java
>  e28e02a 
>   
> flume-ng-sources/flume-jms-source/src/test/java/org/apache/flume/source/jms/TestJMSMessageConsumer.java
>  9bace82 
>   
> flume-ng-sources/flume-jms-source/src/test/java/org/apache/flume/source/jms/TestJMSSource.java
>  5423f8f 
>   
> flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/KafkaSourceEmbeddedKafka.java
>  affac03 
>   
> flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/KafkaSourceEmbeddedZookeeper.java
>  db144c2 
>   
> flume-ng-sources/flume-kafka-source/src/test/java/org/apache/flume/source/kafka/TestKafkaSource.java
>  b4250de 
>   
> flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirEventReader.java
>  1896883 
>   
> flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirMatcher.java
>  4bff841 
>   
> flume-ng-sources/flume-taildir-source/src/test/java/org/apache/flume/source/taildir/TestTaildirSource.java
>  f6289cd 
>   
> flume-ng-tests/src/test/java/org/apache/flume/test/agent/TestFileChannel.java 
> 4a80b8c 
>   flume-ng-tests/src/test/java/org/apache/flume/test/util/StagedInstall.java 
> 973ff4a 
>   flume-ng-tests/src/test/java/org/apache/flume/test/util/SyslogAgent.java 
> 7159549 
>   
> flume-tools/src/test/java/org/apache/flume/tools/TestFileChannelIntegrityTool.java
>  a11126d 
>   pom.xml 85c0dc8 
> 
> Diff: https://reviews.apache.org/r/49830/diff/
> 
> 
> Testing
> -------
> 
> Same as FLUME-2937.
> 
> The primary approach I used to testing this patch was to strip the line 
> numbers from the compiled class files and compare the before-and-after built 
> JARs (the classes inside) at the bytecode level using an MD5 checksum.
> 
> The results showed that, after applying this patch and running ProGuard to 
> strip the line numbers from the generated code, the test class files matched 
> exactly. That means that this patch is a PURE whitespace patch (as far as the 
> Java code is concerned).
> 
> The specific tool I used is called ProGuard ( 
> http://proguard.sourceforge.net/ ). ProGuard is a Java bytecode obfuscator 
> and optimizer used in the Android ecosystem. I posted the configuration file 
> and scripts I used to run ProGuard on my GitHub account (I don't see a reason 
> to keep this as part of Flume proper): 
> https://github.com/mpercy/flume/commit/8365045eb0c525e59e2f2eea5e2aeca1fd5e6220
> 
> I backed out several changes caught by this process so that all of the 
> generated class files are the same, and I added several checkstyle 
> suppressions that we can remove with code changes later.
> 
> I also ran all of the unit tests. The flaky ones are still flaky, but the 
> rest passed.
> 
> It's worth mentioning that the ProGuard config file I used displayed a lot of 
> warnings, which leads me to believe that Flume is not written so that it is 
> fully ProGuard compatible (it uses a lot of dynamic loading, for example) so 
> it's likely that if we actually wanted to use ProGuard to obfuscate or strip 
> Flume that we would need to make more code modifications. For the purpose I 
> used it for, however, I believe that it is reliable.
> 
> 
> Thanks,
> 
> Mike Percy
> 
>

Reply via email to