[
https://issues.apache.org/jira/browse/HADOOP-18155?focusedWorklogId=738299&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-738299
]
ASF GitHub Bot logged work on HADOOP-18155:
-------------------------------------------
Author: ASF GitHub Bot
Created on: 08/Mar/22 19:47
Start Date: 08/Mar/22 19:47
Worklog Time Spent: 10m
Work Description: steveloughran commented on a change in pull request
#4053:
URL: https://github.com/apache/hadoop/pull/4053#discussion_r822005739
##########
File path:
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java
##########
@@ -640,7 +635,7 @@ public void testUnTar() throws IOException {
assertTrue(regularFile.exists());
try {
FileUtil.unTar(simpleTar, regularFile);
- assertTrue("An IOException expected.", false);
+ fail("An IOException expected.");
Review comment:
move to
```
LambdaTestUtils.intercept(IOException.class, () ->
FileUtil.unTar(simpleTar, regularFile));
```
##########
File path:
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java
##########
@@ -294,7 +292,7 @@ public void testFullyDeleteSymlinks() throws IOException {
ret = FileUtil.fullyDelete(linkDir);
Assert.assertTrue(ret);
Assert.assertFalse(linkDir.exists());
- Assert.assertEquals(3, del.list().length);
+ Assert.assertEquals(3, Objects.requireNonNull(del.list()).length);
Review comment:
i'd be tempted to move to AssertJ here, e.g
```
Assertions.assertThat(del.list)
.describedAs("del list")
.isNotNulll()
.hasLength(3)
```
actually, given how many places it us used, I'd factor this out into its
own assertion used
through all the tests
##########
File path:
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java
##########
@@ -160,7 +162,7 @@ public void setup() throws IOException {
new File(del, FILE).createNewFile();
File tmpFile = new File(tmp, FILE);
- tmpFile.createNewFile();
+ assertTrue(tmpFile.createNewFile());
Review comment:
there's enough use of the same file operations (createNewFile, mkdirs,
delete, exists..) that they could be factored out into helper methods which
include the assertion the return value *and* meaningful exception error messages
##########
File path:
hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java
##########
@@ -1419,7 +1443,7 @@ private void putEntriesInTar(TarArchiveOutputStream tos,
File f)
if (f.isDirectory()) {
tos.putArchiveEntry(new TarArchiveEntry(f));
tos.closeArchiveEntry();
- for (File child : f.listFiles()) {
+ for (File child : Objects.requireNonNull(f.listFiles())) {
Review comment:
this will throw an NPE anyway; no need for the extra check
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 738299)
Time Spent: 1h (was: 50m)
> Refactor tests in TestFileUtil
> ------------------------------
>
> Key: HADOOP-18155
> URL: https://issues.apache.org/jira/browse/HADOOP-18155
> Project: Hadoop Common
> Issue Type: Improvement
> Components: common
> Affects Versions: 3.4.0
> Reporter: Gautham Banasandra
> Assignee: Gautham Banasandra
> Priority: Trivial
> Labels: pull-request-available
> Time Spent: 1h
> Remaining Estimate: 0h
>
> We need to ensure that we check the results of file operations whenever we
> invoke *mkdir*, *deleteFile* etc. and assert them right there before
> proceeding on. Also, we need to ensure that some of the relevant FileSystem
> APIs don't return null.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
