You are viewing a plain text version of this content. The canonical link for it is here.
Posted to common-dev@hadoop.apache.org by vinayakumarb <gi...@git.apache.org> on 2016/04/03 11:23:24 UTC
[GitHub] hadoop pull request: HADOOP-12984. Add GenericTestUtils.getTestDir...
GitHub user vinayakumarb opened a pull request:
https://github.com/apache/hadoop/pull/89
HADOOP-12984. Add GenericTestUtils.getTestDir method and use it for emporary directory in tests
Rebased.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/vinayakumarb/hadoop features/HADOOP-12984
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/hadoop/pull/89.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #89
----
commit ba071f9f0bb5b9133a0443653fa03f29b77e2624
Author: Vinayakumar B <vi...@apache.org>
Date: 2016-04-01T13:31:54Z
HADOOP-12984. Add GenericTestUtils.getTestDir method and use it for temporary directory in tests
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] hadoop pull request: HADOOP-12984. Add GenericTestUtils.getTestDir...
Posted by aajisaka <gi...@git.apache.org>.
Github user aajisaka commented on a diff in the pull request:
https://github.com/apache/hadoop/pull/89#discussion_r58492524
--- Diff: hadoop-common-project/hadoop-common/src/test/java/org/apache/hadoop/fs/TestFileUtil.java ---
@@ -55,9 +56,11 @@
public class TestFileUtil {
private static final Log LOG = LogFactory.getLog(TestFileUtil.class);
- private static final String TEST_ROOT_DIR = System.getProperty(
- "test.build.data", "/tmp") + "/fu";
+ private static final String TEST_ROOT_DIR =
+ new File(GenericTestUtils.getTestDir(), "fu").getAbsolutePath();
private static final File TEST_DIR = new File(TEST_ROOT_DIR);
+ private static final String cacheDir = System.getProperty("test.cache.data",
+ new File(TEST_DIR, "cache").getAbsolutePath());
--- End diff --
* The code creates String by `File.getAbsolutePath()` and then creates File by `new File(String)`. It can be simplified by the following and `TEST_ROOT_DIR` can be removed.
```
private static final File TEST_DIR = new File(GenericTestUtils.getTestDir(), "fu");
```
* `cacheDir` is unused.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] hadoop pull request: HADOOP-12984. Add GenericTestUtils.getTestDir...
Posted by aajisaka <gi...@git.apache.org>.
Github user aajisaka commented on a diff in the pull request:
https://github.com/apache/hadoop/pull/89#discussion_r58334727
--- Diff: hadoop-hdfs-project/hadoop-hdfs/src/test/java/org/apache/hadoop/hdfs/server/namenode/TestDecommissioningStatus.java ---
@@ -87,7 +87,7 @@ public static void setUp() throws Exception {
// Set up the hosts/exclude files.
localFileSys = FileSystem.getLocal(conf);
Path workingDir = localFileSys.getWorkingDirectory();
- dir = new Path(workingDir, "build/test/data/work-dir/decommission");
+ dir = new Path(workingDir, "target/test/data" + "work-dir/decommission");
--- End diff --
I'm thinking the string concatenation by `+` is unnecessarily.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] hadoop pull request: HADOOP-12984. Add GenericTestUtils.getTestDir...
Posted by aajisaka <gi...@git.apache.org>.
Github user aajisaka commented on the pull request:
https://github.com/apache/hadoop/pull/89#issuecomment-205680037
Thank you for the update! I'm +1 if the above comments are addressed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] hadoop pull request: HADOOP-12984. Add GenericTestUtils.getTestDir...
Posted by vinayakumarb <gi...@git.apache.org>.
Github user vinayakumarb commented on the pull request:
https://github.com/apache/hadoop/pull/89#issuecomment-205285404
Rebased,
and separated the common changes and hdfs changes.
Pushed only common changes in this PR.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] hadoop pull request: HADOOP-12984. Add GenericTestUtils.getTestDir...
Posted by aajisaka <gi...@git.apache.org>.
Github user aajisaka commented on the pull request:
https://github.com/apache/hadoop/pull/89#issuecomment-205168263
Thank you for rebasing! Mostly looks good to me. Two comments:
* Can we separate the patch into common change and hdfs change? That way the patch becomes smaller and the review becomes easier.
* I'm thinking we should replace not only `test.build.data` but also `test.build.dir` with `GenericTestUtils.getTestDir`. I'm okay with doing this by separate jira(s).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---