> On June 20, 2018, 10:56 a.m., András Piros wrote:
> > core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutorLibAddition.java
> > Lines 433-441 (patched)
> > <https://reviews.apache.org/r/66656/diff/3/?file=2042344#file2042344line433>
> >
> > Extract method `checkFilesInClassPath(final Path[] baseClasspath, final
> > Path[] filesToCheck, final Predicate<Boolean> assertion)`
>
> Mate Juhasz wrote:
> Could you help me with an example how to use 'Predicate<Boolean>
> assertion' in this case?
>
> András Piros wrote:
> It's even simpler than that :) a `boolean` would suffice.
>
> Here is the complete refactor:
> ```
>
> private void verifyFilesInDistributedCache(Map<String, Path> libs,
> Configuration jobConf) throws IOException {
> assertTrue(DistributedCache.getSymlink(jobConf));
>
> Path[] baseClassPath = {new Path(getAppPath(), TEST_JAR),
> libs.get(TEST_ROOT_JAR)};
> Path[] filesToCheck = DistributedCache.getFileClassPaths(jobConf);
> boolean shouldBePresent = true;
> checkFilesInClasspath(baseClassPath, filesToCheck,
> shouldBePresent);
>
> baseClassPath = new Path[]{new Path(getAppPath(), TEST_FILE),
> libs.get(TEST_ROOT_FILE),
> new Path(getAppPath(), TEST_FILE_SO),
> libs.get(TEST_ROOT_SO),
> new Path(getAppPath(), TEST_FILE_SO_1),
> libs.get(TEST_ROOT_SO_1)};
> shouldBePresent = false;
> checkFilesInClasspath(baseClassPath, filesToCheck,
> shouldBePresent);
>
> baseClassPath = new Path[]{new Path(getAppPath(), TEST_JAR),
> libs.get(TEST_ROOT_JAR),
> new Path(getAppPath(), TEST_FILE),
> libs.get(TEST_ROOT_FILE),
> new Path(getAppPath(), TEST_FILE_SO),
> libs.get(TEST_ROOT_SO),
> new Path(getAppPath(), TEST_FILE_SO_1),
> libs.get(TEST_ROOT_SO_1)};
> filesToCheck =
> urisToPaths(DistributedCache.getCacheFiles(jobConf));
> shouldBePresent = true;
> checkFilesInClasspath(baseClassPath, filesToCheck,
> shouldBePresent);
>
> baseClassPath = new Path[]{new Path(getAppPath(), TEST_ARCHIVE),
> libs.get(TEST_ROOT_ARCHIVE)};
> filesToCheck =
> urisToPaths(DistributedCache.getCacheArchives(jobConf));
> checkFilesInClasspath(baseClassPath, filesToCheck,
> shouldBePresent);
> }
>
> private void checkFilesInClasspath(final Path[] baseClasspath, final
> Path[] filesToCheck, final boolean shouldBePresent) {
> for (final Path basePath : baseClasspath) {
> boolean found = false;
> for (final Path fileToCheck : filesToCheck) {
> if (!found &&
> basePath.toUri().getPath().equals(fileToCheck.toUri().getPath())) {
> found = true;
> }
> }
> if (shouldBePresent) {
> assertTrue("file " + basePath.toUri().getPath() + " not
> found in classpath", found);
> }
> else {
> assertFalse("file " + basePath.toUri().getPath() + "
> found in classpath", found);
> }
> }
>
> }
> ```
>
> Isn't it way nicer? ;)
Oh sorry, the complete refactor again:
```
private void verifyFilesInDistributedCache(Map<String, Path> libs,
Configuration jobConf) throws IOException {
assertTrue(DistributedCache.getSymlink(jobConf));
Path[] baseClassPath = {new Path(getAppPath(), TEST_JAR),
libs.get(TEST_ROOT_JAR)};
Path[] filesToCheck = DistributedCache.getFileClassPaths(jobConf);
boolean shouldBePresent = true;
checkFilesInClasspath(baseClassPath, filesToCheck, shouldBePresent);
baseClassPath = new Path[]{new Path(getAppPath(), TEST_FILE),
libs.get(TEST_ROOT_FILE),
new Path(getAppPath(), TEST_FILE_SO), libs.get(TEST_ROOT_SO),
new Path(getAppPath(), TEST_FILE_SO_1),
libs.get(TEST_ROOT_SO_1)};
shouldBePresent = false;
checkFilesInClasspath(baseClassPath, filesToCheck, shouldBePresent);
baseClassPath = new Path[]{new Path(getAppPath(), TEST_JAR),
libs.get(TEST_ROOT_JAR),
new Path(getAppPath(), TEST_FILE), libs.get(TEST_ROOT_FILE),
new Path(getAppPath(), TEST_FILE_SO), libs.get(TEST_ROOT_SO),
new Path(getAppPath(), TEST_FILE_SO_1),
libs.get(TEST_ROOT_SO_1)};
filesToCheck = urisToPaths(DistributedCache.getCacheFiles(jobConf));
shouldBePresent = true;
checkFilesInClasspath(baseClassPath, filesToCheck, shouldBePresent);
baseClassPath = new Path[]{new Path(getAppPath(), TEST_ARCHIVE),
libs.get(TEST_ROOT_ARCHIVE)};
filesToCheck = urisToPaths(DistributedCache.getCacheArchives(jobConf));
checkFilesInClasspath(baseClassPath, filesToCheck, shouldBePresent);
}
private void checkFilesInClasspath(final Path[] baseClasspath, final Path[]
filesToCheck, final boolean shouldBePresent) {
for (final Path basePath : baseClasspath) {
boolean found = false;
for (final Path fileToCheck : filesToCheck) {
if (!found &&
basePath.toUri().getPath().equals(fileToCheck.toUri().getPath())) {
found = true;
}
}
if (shouldBePresent) {
assertTrue("file " + basePath.toUri().getPath() + " not found
in classpath", found);
}
else {
assertFalse("file " + basePath.toUri().getPath() + " found in
classpath", found);
}
}
}
private Path[] urisToPaths(final URI[] uris) {
final Path[] paths = new Path[uris.length];
for (int ixUri = 0; ixUri < uris.length; ixUri++) {
paths[ixUri] = new Path(uris[ixUri].getPath());
}
return paths;
}
```
- András
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/66656/#review205055
-----------------------------------------------------------
On June 19, 2018, 3:23 p.m., Mate Juhasz wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/66656/
> -----------------------------------------------------------
>
> (Updated June 19, 2018, 3:23 p.m.)
>
>
> Review request for oozie, András Piros and Denes Bodo.
>
>
> Bugs: OOZIE-1624
> https://issues.apache.org/jira/browse/OOZIE-1624
>
>
> Repository: oozie-git
>
>
> Description
> -------
>
> OOZIE-1624 Exclusion pattern for sharelib.
>
>
> Diffs
> -----
>
> core/src/main/java/org/apache/oozie/action/hadoop/JavaActionExecutor.java
> ed809ef0
> core/src/main/java/org/apache/oozie/action/hadoop/ShareLibExcluder.java
> PRE-CREATION
> core/src/main/java/org/apache/oozie/service/ShareLibService.java a901567d
>
> core/src/test/java/org/apache/oozie/action/hadoop/ActionExecutorTestCase.java
> f39bba2c
>
> core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutor.java
> a31079a4
>
> core/src/test/java/org/apache/oozie/action/hadoop/TestJavaActionExecutorLibAddition.java
> PRE-CREATION
> core/src/test/java/org/apache/oozie/action/hadoop/TestShareLibExcluder.java
> PRE-CREATION
> docs/src/site/twiki/WorkflowFunctionalSpec.twiki 76cbe21e
>
>
> Diff: https://reviews.apache.org/r/66656/diff/3/
>
>
> Testing
> -------
>
> Tested on a cluster
>
>
> Thanks,
>
> Mate Juhasz
>
>
