为了获得高质量的生产代码，仅提供最大的测试覆盖范围还不够。 毫无疑问，为了获得较高的结果，主要的项目代码和测试必须完美地结合在一起。 因此，您需要与主要代码一样注意测试。 编写良好的测试是赶上生产回归的关键。 为了说明测试中的错误不会比生产中的错误严重这一事实的重要性，我们将考虑对PVS-Studio静态分析器中的警告进行下一次分析。 目标：Apache Hadoop。
关于项目
那些曾经对大数据感兴趣的人可能听说过或与
Apache Hadoop等项目合作。 简而言之，Hadoop是一个框架，可以用作构建和使用大数据系统的基础。
Hadoop由四个主要模块组成，每个模块执行大数据分析系统所需的特定任务：
- Hadoop常见
- Mapreduce
- Hadoop分布式文件系统（Hadoop分布式文件系统）
- 纱线
但是，有很多资料可以在Internet上熟悉。
关于验证
如
文档所示，可以通过多种方式将PVS-Studio集成到项目中：
- 使用Maven插件;
- 使用gradle插件;
- 使用IntellJ IDEA
- 直接使用分析仪。
Hadoop是基于Maven构建系统构建的，因此验证没有困难。
从文档中整合了脚本并稍微调整了pom.xml之一（依赖项中没有模块），分析就开始了！
在分析之后，选择最有趣的警告，我注意到在生产代码和测试中，我具有相同数量的警告。 通常，我不认为测试会触发分析仪。 但是，将它们分开，我不会错过“测试”类别的警告。 我想：“为什么不呢？”，因为测试中的错误也会带来后果。 它们可能导致错误或部分测试，甚至导致废话（仅出于演示目的，因此它们始终为绿色）。
因此，我收集了最有趣的警告，并按代码（生产，测试）和四个主要的Hadoop模块进行了划分，请您注意分析器操作的分析。
生产代码
Hadoop常见
V6033已经添加了具有相同键“ KDC_BIND_ADDRESS”的项目。 MiniKdc.java（163），MiniKdc.java（162）
public class MiniKdc { .... private static final Set<String> PROPERTIES = new HashSet<String>(); .... static { PROPERTIES.add(ORG_NAME); PROPERTIES.add(ORG_DOMAIN); PROPERTIES.add(KDC_BIND_ADDRESS); PROPERTIES.add(KDC_BIND_ADDRESS);
检查项目时，向
HashSet两次添加的值是常见缺陷。 实际上，第二个添加项将被忽略。 好吧，如果这种重复是荒谬的事故。 但是，如果真的意味着要增加另一个价值呢？
Mapreduce
V6072找到两个相似的代码片段。 也许这是一个错字，应该使用“ localFiles”变量而不是“ localArchives”。 LocalDistributedCacheManager.java（183），LocalDistributedCacheManager.java（178），LocalDistributedCacheManager.java（176），LocalDistributedCacheManager.java（181）
public synchronized void setup(JobConf conf, JobID jobId) throws IOException { ....
诊断V6072有时会产生非常有趣的发现。 诊断的本质是搜索通过复制粘贴并替换一个或两个变量而获得的相同类型的代码片段，但同时某些变量被“低估了”。
上面的代码演示了这一点。 在第一个块中，使用
localArchives变量执行操作，在相同类型的下一个块中，使用
localFiles执行操作 。 而且，如果您认真研究此代码，并且不像代码审查那样经常快速浏览它，那么请注意忘记替换
localArchives变量的位置。
这种疏忽可能导致以下情况：
- 假设我们有localArchives （大小= 4）和localFiles （大小= 2）;
- 当创建localFiles.toArray数组（新的String [localArchives.size（）]）时 ，我们将最后2个元素设置为null （[[pathToFile1“，” pathToFile2“，null，null]）；
- 之后， org.apache.hadoop.util.StringUtils.arrayToString将返回数组的字符串表示形式，其中最后一个文件名将表示为“ null”（“ pathToFile1，pathToFile2，null，null” ） ；
- 所有这些都将继续下去，谁知道这种情况下进行哪些检查=）。
V6007表达式'children.size（）> 0'始终为true。 Queue.java（347）
boolean isHierarchySameAs(Queue newState) { .... if (children == null || children.size() == 0) { .... } else if(children.size() > 0) { .... } .... }
由于要单独检查0处的元素数，因此进一步检查
children.size（）> 0将始终为true。
HDFS
V6001在'％'运算符的左侧和右侧有相同的子表达式'this.bucketSize'。 RollingWindow.java（79）
RollingWindow(int windowLenMs, int numBuckets) { buckets = new Bucket[numBuckets]; for (int i = 0; i < numBuckets; i++) { buckets[i] = new Bucket(); } this.windowLenMs = windowLenMs; this.bucketSize = windowLenMs / numBuckets; if (this.bucketSize % bucketSize != 0) {
该缺陷在于以下事实：变量被分成了自己。 结果，将始终通过多重性检查，并且在输入数据不正确的情况下（
windowLenMs ，
numBuckets ），将不会引发异常。
纱线
V6067两个或更多案件分支执行相同的操作。 TimelineEntityV2Converter.java（386），TimelineEntityV2Converter.java（389）
public static ApplicationReport convertToApplicationReport(TimelineEntity entity) { .... if (metrics != null) { long vcoreSeconds = 0; long memorySeconds = 0; long preemptedVcoreSeconds = 0; long preemptedMemorySeconds = 0; for (TimelineMetric metric : metrics) { switch (metric.getId()) { case ApplicationMetricsConstants.APP_CPU_METRICS: vcoreSeconds = getAverageValue(metric.getValues().values()); break; case ApplicationMetricsConstants.APP_MEM_METRICS: memorySeconds = ....; break; case ApplicationMetricsConstants.APP_MEM_PREEMPT_METRICS: preemptedVcoreSeconds = ....;
在两个
案例分支中，相同的代码片段。 这总是发生！ 在大多数情况下，这不是一个真正的错误，而只是考虑
交换机重构的一个机会。 但并非针对上述情况。 在重复代码段中，
设置了变量
preemptedVcoreSeconds的值。 如果注意所有变量和常量的名称，则可以得出结论，对于
metric.getId（）== APP_MEM_PREEMPT_METRICS ，应设置变量
preemptedMemorySeconds的值，而不是
preemptedVcoreSeconds 。 在这方面，在执行“ switch”语句后，
preemptedMemorySeconds将始终保持为0，并且
preemptedVcoreSeconds的值可能不正确。
V6046格式错误。 预计会有不同数量的格式项。 不使用的参数：2. AbstractSchedulerPlanFollower.java（186）
@Override public synchronized void synchronizePlan(Plan plan, boolean shouldReplan) { .... try { setQueueEntitlement(planQueueName, ....); } catch (YarnException e) { LOG.warn("Exception while trying to size reservation for plan: {}", currResId, planQueueName, e); } .... }
记录时未使用的变量
planQueueName 。 在这里，它们要么复制太多，要么没有修改格式字符串。 但是，尽管如此，我还是倾向于旧的，有时会带来损害的复制粘贴。
测试码
Hadoop常见
V6072找到两个相似的代码片段。 也许这是一个错字，应该使用'allSecretsB'变量而不是'allSecretsA'。 TestZKSignerSecretProvider.java（316），TestZKSignerSecretProvider.java（309），TestZKSignerSecretProvider.java（306），TestZKSignerSecretProvider.java（313）
public void testMultiple(int order) throws Exception { .... currentSecretA = secretProviderA.getCurrentSecret(); allSecretsA = secretProviderA.getAllSecrets(); Assert.assertArrayEquals(secretA2, currentSecretA); Assert.assertEquals(2, allSecretsA.length);
再次是V6072。 注意变量
allSecretsA和
allSecretsB 。
V6043考虑检查“ for”运算符。 迭代器的初始值和最终值相同。 TestTFile.java（235）
private int readPrepWithUnknownLength(Scanner scanner, int start, int n) throws IOException { for (int i = start; i < start; i++) { String key = String.format(localFormatter, i); byte[] read = readKey(scanner); assertTrue("keys not equal", Arrays.equals(key.getBytes(), read)); try { read = readValue(scanner); assertTrue(false); } catch (IOException ie) {
测试始终是绿色的？ =）。 作为测试一部分的循环主体永远不会执行。 这是由于在
for语句中计数器的开始值和结束值匹配。 结果，条件
i <开始将立即为我们提供错误，这将导致这种行为。 我遍历了文件并进行了测试，得出的结论是需要在循环
i <（start + n）的条件下进行写操作。
Mapreduce
a href =“
www.viva64.com/en/w/v6007 ”> V6007表达式'byteAm <0'始终为false。 DataWriter.java（322）
GenerateOutput writeSegment(long byteAm, OutputStream out) throws IOException { long headerLen = getHeaderLength(); if (byteAm < headerLen) {
条件
byteAm <0始终为false。 为了理解，让我们看一下上面的代码。 如果测试执行到达操作
byteAm-= headerLen ，则意味着将存在
byteAm> = headerLen 。 从这里开始，执行减法后，
byteAm的值将永远不会为负。 需要证明。
HDFS
V6072找到两个相似的代码片段。 也许这是一个错字，应该使用'normalFile'变量而不是'normalDir'。 TestWebHDFS.java（625），TestWebHDFS.java（615），TestWebHDFS.java（614），TestWebHDFS.java（624）
public void testWebHdfsErasureCodingFiles() throws Exception { .... final Path normalDir = new Path("/dir"); dfs.mkdirs(normalDir); final Path normalFile = new Path(normalDir, "file.log"); ....
不要相信，再来一次V6072！ 只需遵循
normalDir和
normalFile变量V6027通过调用同一函数来初始化变量。 可能是错误或未优化的代码。 TestDFSAdmin.java（883），TestDFSAdmin.java（879）
private void verifyNodesAndCorruptBlocks( final int numDn, final int numLiveDn, final int numCorruptBlocks, final int numCorruptECBlockGroups, final DFSClient client, final Long highestPriorityLowRedundancyReplicatedBlocks, final Long highestPriorityLowRedundancyECBlocks) throws IOException { .... final String expectedCorruptedECBlockGroupsStr = String.format( "Block groups with corrupt internal blocks: %d", numCorruptECBlockGroups); final String highestPriorityLowRedundancyReplicatedBlocksStr = String.format( "\tLow redundancy blocks with highest priority " + "to recover: %d", highestPriorityLowRedundancyReplicatedBlocks); final String highestPriorityLowRedundancyECBlocksStr = String.format( "\tLow redundancy blocks with highest priority " + "to recover: %d", highestPriorityLowRedundancyReplicatedBlocks); .... }
在此片段中，变量
highestPriorityLowRedundancyReplicatedBlocksStr和
highestPriorityLowRedundancyECBlocksStr用相同的值初始化。 通常应该如此，但在这种情况下不应该如此。 此处的变量名称很长且彼此相似，因此对于复制粘贴没有进行相应的修改，我并不感到惊讶。 为了纠正这种情况，在初始化变量
highestPriorityLowRedundancyECBlocksStr时，需要使用输入参数
highestPriorityLowRedundancyECBlocks 。 除此之外，最有可能的是，您仍然需要修复格式字符串。
V6019检测
不到代码。 可能存在错误。 TestReplaceDatanodeFailureReplication.java（222）
private void verifyFileContent(...., SlowWriter[] slowwriters) throws IOException { LOG.info("Verify the file"); for (int i = 0; i < slowwriters.length; i++) { LOG.info(slowwriters[i].filepath + ....); FSDataInputStream in = null; try { in = fs.open(slowwriters[i].filepath); for (int j = 0, x;; j++) { x = in.read(); if ((x) != -1) { Assert.assertEquals(j, x); } else { return; } } } finally { IOUtils.closeStream(in); } } }
分析器发誓，无法在循环中更改
i ++计数器。 这意味着在
for循环中
（int i = 0; i <slowwriters.length; i ++）{....}将仅执行一次迭代。 让我们找出原因。 因此，在第一次迭代中，我们将流与
慢速写入器[0]对应的文件相关联，以供进一步读取。 通过
for循环
（int j = 0，x ;; j ++），我们按字节读取文件的内容，其中：
- 如果我们读取了足够的东西，则通过assertEquals将读取的字节与计数器j的当前值进行比较（如果验证失败，则以失败退出测试）；
- 如果文件通过了测试并且我们到达了文件的末尾（读取为-1），则退出该方法。
因此，无论检查
慢写程序[0]时发生什么情况，都不会验证以下元素。 最有可能的是，应该使用
break而不是
return 。
纱线
V6019检测
不到代码。 可能存在错误。 TestNodeManager.java（176）
@Test public void testCreationOfNodeLabelsProviderService() throws InterruptedException { try { .... } catch (Exception e) { Assert.fail("Exception caught"); e.printStackTrace(); } }
在这种情况下，如果发生异常，将永远不会打印stacktrace，因为
Assert.fail方法将中断测试。 如果有足够的消息捕获到异常，那么为了不引起混淆，需要删除stacktrace'a打印。 如果需要打印，则只需交换它们。
有很多这样的地方：
- V6019检测不到代码。 可能存在错误。 TestResourceTrackerService.java（928）
- V6019检测不到代码。 可能存在错误。 TestResourceTrackerService.java（737）
- V6019检测不到代码。 可能存在错误。 TestResourceTrackerService.java（685）
- ....
V6072找到两个相似的代码片段。 也许这是一个错字，应该使用“ publicCache”变量而不是“ usercache”。 TestResourceLocalizationService.java（315），TestResourceLocalizationService.java（309），TestResourceLocalizationService.java（307），TestResourceLocalizationService.java（313）
@Test public void testDirectoryCleanupOnNewlyCreatedStateStore() throws IOException, URISyntaxException { ....
最后，再次是V6072 =）。 使您熟悉可疑代码段的变量：
usercache和
publicCache 。
结论
在开发过程中，编写了数十万行代码。 如果生产代码试图清除错误，缺陷和缺点（开发人员测试自己的代码，进行代码审查等等），则测试显然不如此。 测试中的缺陷可以悄悄地隐藏在“绿色刻度”后面。 从今天对警告的分析中可以了解到，成功通过测试并非总是可以保证的测试。
在检查Apache Hadoop代码库时，静态分析表明它不仅需要投入生产的代码，还需要对在开发中起重要作用的测试的需求。
因此，如果您关心代码和测试库的质量，那么我建议您考虑进行静态分析。 我是测试的第一位申请人，我建议尝试
PVS-Studio 。
如果您想与讲英语的人分享这篇文章，请使用以下链接：Maxim Stefanov。
Apache Hadoop代码质量：生产VS测试 。