> On July 14, 2015, 2:49 a.m., Guozhang Wang wrote: > > Some general comments: > > > > 1. Regarding the @Before and @After annotations, one suggestion from the > > JIRA was that we remove any annotations other than "@Test" itself but use > > scalatest features (for example, > > http://doc.scalatest.org/2.2.4/#org.scalatest.BeforeAndAfter) instead. Now > > I cannot remember a strong motiviation for this move, so I feel it may be > > also OK as you chose to use the junit tags anyways. > > 2. Regarding org.junit.Assert and org.scalatest.Assertions in imports, if > > we decide to be junit-heavy instead of scalatest-heavy for our unit tests, > > we should then use the former for most of the time and only the latter for > > intercept[..] since it is not supported in the fomer. There seems a few > > places where both of them are used for fail / assert, etc.
1. We could only use BeforeAndAfter for classes that aren't going to extended, because this trair isn't stackable. I'll go ahead and try BeforeAndAfter each and report back with results. 2. Totally agree to keep code using only one. > On July 14, 2015, 2:49 a.m., Guozhang Wang wrote: > > core/src/test/scala/integration/kafka/api/ConsumerTest.scala, line 33 > > <https://reviews.apache.org/r/35615/diff/2/?file=987171#file987171line33> > > > > Seems this import is not used inside the class? I've found what was the root cause of this import I've accidentaly removed JUnit3Suite in KafkaServerTestHarness instead of replacing it with JUnitSuite. I'm going to remove unnecessary imports > On July 14, 2015, 2:49 a.m., Guozhang Wang wrote: > > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala, line 34 > > <https://reviews.apache.org/r/35615/diff/2/?file=987176#file987176line34> > > > > Sometimes we use org.scalatest.Assertions.fail() and sometimes we use > > org.junit.Assert.fail(); it would better that we are consistent in one of > > them, and personally I recommend following the org.junit package since it > > is more general. Fixed to use org.junit.* only - Alexander ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/35615/#review91575 ----------------------------------------------------------- On July 15, 2015, 11:58 p.m., Alexander Pakulov wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/35615/ > ----------------------------------------------------------- > > (Updated July 15, 2015, 11:58 p.m.) > > > Review request for kafka. > > > Bugs: KAFKA-1782 > https://issues.apache.org/jira/browse/KAFKA-1782 > > > Repository: kafka > > > Description > ------- > > Merge branch 'trunk' into KAFKA-1782 > > > Merge branch 'trunk' into KAFKA-1782 > > > KAFKA-1782; Cleanups > > > KAFKA-1782; Cleanups > > > Diffs > ----- > > core/src/test/scala/integration/kafka/api/ConsumerBounceTest.scala > b0750faa43dfe21a9226335020b4b5dac6cf65bf > core/src/test/scala/integration/kafka/api/ConsumerTest.scala > 92ffb91b5e039dc0d4cd0e072ca46db32f280cf9 > core/src/test/scala/integration/kafka/api/IntegrationTestHarness.scala > afcc349342d44f42e19ca730dbd074c338638a64 > core/src/test/scala/integration/kafka/api/ProducerBounceTest.scala > ce70a0a449883723a9b59ea48da34ba30b3f6daf > core/src/test/scala/integration/kafka/api/ProducerCompressionTest.scala > 83de81cb3f79a6966dd5ef462733d8a22cd6d467 > core/src/test/scala/integration/kafka/api/ProducerFailureHandlingTest.scala > ee94011894b46864614b97bbd2a98375a7d3f20b > core/src/test/scala/integration/kafka/api/ProducerSendTest.scala > 9ce4bd5ee130ce3cb252b2883a3fd3c9acd742a5 > core/src/test/scala/unit/kafka/KafkaConfigTest.scala > 4764c8976022dfaaebd7ef1cd4ddca55a3ed8a89 > core/src/test/scala/unit/kafka/admin/AddPartitionsTest.scala > df5c6ba20f01e497ce896af790cbab40369f1776 > core/src/test/scala/unit/kafka/admin/AdminTest.scala > 252ac813c8df1780c2dc5fa9e698fb43bb6d5cf8 > core/src/test/scala/unit/kafka/admin/DeleteConsumerGroupTest.scala > 1913ad6d3e6eb29a0c939c4f59f6b688c8c925fa > core/src/test/scala/unit/kafka/admin/DeleteTopicTest.scala > fa8ce259a2832ab86f9dda8c1d409b2c42d43ae9 > core/src/test/scala/unit/kafka/admin/TopicCommandTest.scala > dcd69881445c29765f66a7d21d2d18437f4df428 > core/src/test/scala/unit/kafka/api/ApiUtilsTest.scala > 255442526d94157b7a0b5a92e1d6a900aacb7536 > core/src/test/scala/unit/kafka/api/RequestResponseSerializationTest.scala > 5717165f2344823fabe8f7cfafae4bb8af2d949a > core/src/test/scala/unit/kafka/cluster/BrokerEndPointTest.scala > abe511fc1458bfb5a93c152aed81a827cc24ce68 > core/src/test/scala/unit/kafka/common/ConfigTest.scala > 0aca9385bff093b4cb52e42291c9c1d58d9600de > core/src/test/scala/unit/kafka/common/TopicTest.scala > 79532c89c41572ba953c4dc3319a05354927e961 > core/src/test/scala/unit/kafka/consumer/ConsumerIteratorTest.scala > db5302ff02851f9f1a59419b1e071286bff0e142 > core/src/test/scala/unit/kafka/consumer/PartitionAssignorTest.scala > adf08010597b7c6ed72eddf93962497c3209e10f > core/src/test/scala/unit/kafka/consumer/TopicFilterTest.scala > 4b326d090c9486c812afb2603e94d77a2459d04c > > core/src/test/scala/unit/kafka/consumer/ZookeeperConsumerConnectorTest.scala > 359b0f5d14f82d14ee423cde271bb35b7034766d > > core/src/test/scala/unit/kafka/coordinator/ConsumerCoordinatorResponseTest.scala > 87a5330e716b9cedc9544229e29f42be7150c8fb > core/src/test/scala/unit/kafka/coordinator/ConsumerGroupMetadataTest.scala > b69c993a3fa49f7f01dd28e13ce465c2a89eeba5 > core/src/test/scala/unit/kafka/coordinator/CoordinatorMetadataTest.scala > 2cbf6e251adbfd3fb174d08138a13215c1914566 > core/src/test/scala/unit/kafka/coordinator/PartitionAssignorTest.scala > 887cee5a582b5737ba838920399bb9b24bf22382 > core/src/test/scala/unit/kafka/integration/AutoOffsetResetTest.scala > 139dc9a104c024e35fd9bc5ac9333e6bd208b571 > core/src/test/scala/unit/kafka/integration/FetcherTest.scala > facebd8f81c67f26f41a96bce343227bc9b55893 > core/src/test/scala/unit/kafka/integration/KafkaServerTestHarness.scala > 87c631573aa1e0275d6618c4ac3b33e76fa6abd3 > core/src/test/scala/unit/kafka/integration/MinIsrConfigTest.scala > a2c97134d85c637256440b5eb42f594d50f0cfe4 > core/src/test/scala/unit/kafka/integration/PrimitiveApiTest.scala > 6a758a7db71bdc4794b1e5f264f8ca6d410ff2ba > > core/src/test/scala/unit/kafka/integration/ProducerConsumerTestHarness.scala > 4614a922e739098dbb0ff560d831e26045e32023 > core/src/test/scala/unit/kafka/integration/RollingBounceTest.scala > 12d0733f5edf436315f884bc193da533d9d4a4ee > core/src/test/scala/unit/kafka/integration/TopicMetadataTest.scala > 5b6c9d60d29436036f8287da6bad332c1a3a6ec9 > core/src/test/scala/unit/kafka/integration/UncleanLeaderElectionTest.scala > e4bf2df48dd59a251b646b7f96d63ec4b924fc0b > > core/src/test/scala/unit/kafka/javaapi/consumer/ZookeeperConsumerConnectorTest.scala > 74c761dec7afc98667032bdec8359a9aa7c2ecc2 > > core/src/test/scala/unit/kafka/javaapi/message/BaseMessageSetTestCases.scala > 726399e3c7a4157223b5037ff2a03da51236e876 > > core/src/test/scala/unit/kafka/javaapi/message/ByteBufferMessageSetTest.scala > 383fcef02994fde07e669651e522b9e5ee239dd8 > core/src/test/scala/unit/kafka/log/CleanerTest.scala > 0e2a6a1e8e6d0bc6bed90cdc5bd3cb8e490ed364 > core/src/test/scala/unit/kafka/log/FileMessageSetTest.scala > 02cf66882f4065f3ab8449650b0bd1b0e7525d38 > core/src/test/scala/unit/kafka/log/LogCleanerIntegrationTest.scala > 381e9aa6fe611887cd144b4731f9ac351d12bbf1 > core/src/test/scala/unit/kafka/log/LogConfigTest.scala > 19dcb47f3f406b8d6c3668297450ab6b534e4471 > core/src/test/scala/unit/kafka/log/LogManagerTest.scala > a13f2bef8ee8c3d42192c9a60df092023e4d2ff9 > core/src/test/scala/unit/kafka/log/LogSegmentTest.scala > abcd1f07444c6ef1e42bb6db2d92e3aa84e1042e > core/src/test/scala/unit/kafka/log/LogTest.scala > 9e26190d99537b227e2119cb01040739b03cb74c > core/src/test/scala/unit/kafka/log/OffsetIndexTest.scala > 9213a5d9e95d0a5f3009b2cf50baa99765e4cfef > core/src/test/scala/unit/kafka/log/OffsetMapTest.scala > 12ce39e665afd4cb1d8aa8d7d4d7df18b219a141 > core/src/test/scala/unit/kafka/message/BaseMessageSetTestCases.scala > dd8847f5f709a0a7b2e7037d5beda9a0dfc054d6 > core/src/test/scala/unit/kafka/message/ByteBufferMessageSetTest.scala > 07bc317bcd40cf40fbff1cbd5039672508f293d9 > core/src/test/scala/unit/kafka/message/MessageCompressionTest.scala > 76987d4fa68fd39cf0ce6bc47b05eb32b17bbedb > core/src/test/scala/unit/kafka/message/MessageTest.scala > 11c0f817ca06e4a53bce1009ad6a36aa007cf93a > core/src/test/scala/unit/kafka/message/MessageWriterTest.scala > b08a3432ad1ae49372e4b6cea9b247f6be2889da > core/src/test/scala/unit/kafka/metrics/KafkaTimerTest.scala > 7df74050f99567395d4d01c1600ca77cd917652e > core/src/test/scala/unit/kafka/metrics/MetricsTest.scala > b42101b85fa4aa6a2249e7b04ec0f61e51c81c3f > core/src/test/scala/unit/kafka/network/SocketServerTest.scala > 7dc2fad542ea553ee888543dd215eb41ea57d509 > core/src/test/scala/unit/kafka/producer/AsyncProducerTest.scala > be4bb878dc49f05caf55b36e9218ed19b1c56253 > core/src/test/scala/unit/kafka/producer/ProducerTest.scala > 4d2536b462c030f4f269bad024847839e63337c2 > core/src/test/scala/unit/kafka/producer/SyncProducerTest.scala > 8c3fb7a393db019a29b284b26670f63c25d6a4c6 > core/src/test/scala/unit/kafka/server/AdvertiseBrokerTest.scala > e899b023b3153542f481b82e92b258e4595b23dc > core/src/test/scala/unit/kafka/server/DelayedOperationTest.scala > f3ab3f4ff8eb1aa6b2ab87ba75f72eceb6649620 > core/src/test/scala/unit/kafka/server/DynamicConfigChangeTest.scala > 8a871cfaf6a534acd1def06a5ac95b5c985b024c > core/src/test/scala/unit/kafka/server/HighwatermarkPersistenceTest.scala > 60cd8249e6ec03349e20bb0a7226ea9cd66e6b17 > core/src/test/scala/unit/kafka/server/ISRExpirationTest.scala > 90529faf11dca65c3ef6bcb27ad72557bc36f939 > core/src/test/scala/unit/kafka/server/KafkaConfigConfigDefTest.scala > 98a5b042a710d3c1064b0379db1d152efc9eabee > core/src/test/scala/unit/kafka/server/KafkaConfigTest.scala > d3544526b66ce1f146f6986401299da9ebe49cd4 > core/src/test/scala/unit/kafka/server/LeaderElectionTest.scala > f1977d850a5bf1125260949101fa3485b29bd4e6 > core/src/test/scala/unit/kafka/server/LogOffsetTest.scala > e57c1dec2dee4b9a64ffe5906d12676fd877319b > core/src/test/scala/unit/kafka/server/LogRecoveryTest.scala > 7688f26fe42c4481fe2da7a1d50459805be0ebc1 > core/src/test/scala/unit/kafka/server/OffsetCommitTest.scala > 39a68526c8b8569e5ed10d61cbba804c9444e252 > core/src/test/scala/unit/kafka/server/ReplicaFetchTest.scala > a3a03db88c4c3b2949c4ff7f26f2f00498884b18 > core/src/test/scala/unit/kafka/server/ReplicaManagerTest.scala > 00d59337a99ac135e8689bd1ecd928f7b1423d79 > core/src/test/scala/unit/kafka/server/ServerGenerateBrokerIdTest.scala > 12269cde06aab953dcd898821a8afaa5c4290e77 > core/src/test/scala/unit/kafka/server/ServerShutdownTest.scala > 95534e36c29023565fedf6bf0fe537ef270c2420 > core/src/test/scala/unit/kafka/server/ServerStartupTest.scala > 60e10b3d5adda152a01425d98d45ca373a63bebd > core/src/test/scala/unit/kafka/server/SimpleFetchTest.scala > 09a0961f73969aca3b054ed3ff6b181a6500364a > core/src/test/scala/unit/kafka/utils/ByteBoundedBlockingQueueTest.scala > fd8cf7bf04c5f8e6166f1e3dd5ab369b478c02ee > core/src/test/scala/unit/kafka/utils/CommandLineUtilsTest.scala > 6380b6e623e7a6fcaf7b44473291075a0aa010a8 > core/src/test/scala/unit/kafka/utils/IteratorTemplateTest.scala > fbd245cad0afab146354fdcc76193d784e1997d9 > core/src/test/scala/unit/kafka/utils/JsonTest.scala > 93550e8f24071f88eb1ea5b41373efee27e4b8b7 > core/src/test/scala/unit/kafka/utils/ReplicationUtilsTest.scala > b9de8d677a46e1bf37ae0bd4ca0f8b1bfac8d437 > core/src/test/scala/unit/kafka/utils/SchedulerTest.scala > cfea63b88e2590cbd7a1e7cb2c8cacf054bd5568 > core/src/test/scala/unit/kafka/utils/TestUtils.scala > 17e9fe4c159a29033fe9a287db6ced2fdc3fa9d1 > core/src/test/scala/unit/kafka/utils/timer/TimerTaskListTest.scala > 052aecdc32ae14afe6aabf942ed7c0028ed9f979 > core/src/test/scala/unit/kafka/utils/timer/TimerTest.scala > 8507592d1f374248cc7f56ef786544562ca8ad9c > core/src/test/scala/unit/kafka/zk/ZKEphemeralTest.scala > 2be161985657293e83a3fd4d3d15faa30e9a119e > core/src/test/scala/unit/kafka/zk/ZKPathTest.scala > d3e44c62e272b01edd51553270e7d667e55f6e44 > core/src/test/scala/unit/kafka/zk/ZooKeeperTestHarness.scala > 1f4d10d25ab3b907689ead7701d2c868ba703952 > > Diff: https://reviews.apache.org/r/35615/diff/ > > > Testing > ------- > > > Thanks, > > Alexander Pakulov > >
