[ 
https://issues.apache.org/jira/browse/HADOOP-14170?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15907347#comment-15907347
 ] 

Steve Loughran commented on HADOOP-14170:
-----------------------------------------

LGTM, +1

Reviewing this I realised that I'd got confused about tests. 
{{FileSystemContractBaseTest}} is the old JUnit 3.x test suite, not the newer 
AbstractFSContractTestBase tests. That's fairly old piece of code I've always 
left alone fearing that external projects had subclassed it and so we couldn't 
break it. 

your patch is the first bit of long-needed maintenance it's had for a while.

This makes me thing: it's time to move this to JUnit 4, annotate all tests with 
@test, and make the test cases skip if they don't have the test FS defined. 
JUnit 3 doesn't support Assume, so when I do test runs without the s3n or s3 fs 
specced, I get lots of errors I just ignore. 

What do you think? Move to Junit 4, and, in our own code, find everywhere we've 
subclassed a method to make the test a no-op, and insert an 
Assume.assumeTrue(false) in there so they skip properly?

> FileSystemContractBaseTest is not cleaning up test directory clearly
> --------------------------------------------------------------------
>
>                 Key: HADOOP-14170
>                 URL: https://issues.apache.org/jira/browse/HADOOP-14170
>             Project: Hadoop Common
>          Issue Type: Bug
>          Components: fs
>            Reporter: Mingliang Liu
>            Assignee: Mingliang Liu
>         Attachments: HADOOP-14170.000.patch, HADOOP-14170.001.patch, 
> HADOOP-14170.002.patch, HADOOP-14170.003.patch
>
>
> In {{FileSystemContractBaseTest::tearDown()}} method, it cleans up the 
> {{path("/test")}} directory, which will be qualified as {{/test}} (against 
> root instead of working directory because it's absolute):
> {code}
>   @Override
>   protected void tearDown() throws Exception {
>     try {
>       if (fs != null) {
>         fs.delete(path("/test"), true);
>       }
>     } catch (IOException e) {
>       LOG.error("Error deleting /test: " + e, e);
>     }
>   }
> {code}
> But in the test, it uses {{path("test")}} sometimes, which will be made 
> qualified against the working directory (e.g. {{/user/bob/test}}).
> This makes some tests fail intermittently, e.g. 
> {{ITestS3AFileSystemContract}}. Also see the discussion in [HADOOP-13934].



--
This message was sent by Atlassian JIRA
(v6.3.15#6346)

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to