szilard-nemeth commented on code in PR #4439:
URL: https://github.com/apache/hadoop/pull/4439#discussion_r897929690
##########
hadoop-yarn-project/hadoop-yarn/hadoop-yarn-common/src/test/java/org/apache/hadoop/yarn/logaggregation/TestAggregatedLogDeletionService.java:
##########
@@ -138,82 +97,35 @@ public void testDeletion() throws Exception {
long toKeepTime = now - (1500 * 1000);
Configuration conf = setupConfiguration(1800, -1);
-
- Path rootPath = new Path(ROOT);
- FileSystem rootFs = rootPath.getFileSystem(conf);
- FileSystem mockFs = ((FilterFileSystem)rootFs).getRawFileSystem();
-
- Path remoteRootLogPath = new Path(REMOTE_ROOT_LOG_DIR);
- PathWithFileStatus userDir =
createDirLogPathWithFileStatus(remoteRootLogPath, USER_ME,
- toKeepTime);
-
- when(mockFs.listStatus(remoteRootLogPath)).thenReturn(new
FileStatus[]{userDir.fileStatus});
-
- ApplicationId appId1 = ApplicationId.newInstance(now, 1);
- ApplicationId appId2 = ApplicationId.newInstance(now, 2);
- ApplicationId appId3 = ApplicationId.newInstance(now, 3);
- ApplicationId appId4 = ApplicationId.newInstance(now, 4);
-
- PathWithFileStatus suffixDir =
createDirLogPathWithFileStatus(userDir.path, NEW_SUFFIX,
- toDeleteTime);
- PathWithFileStatus bucketDir =
createDirLogPathWithFileStatus(remoteRootLogPath, SUFFIX,
- toDeleteTime);
-
- PathWithFileStatus app1 =
createPathWithFileStatusForAppId(remoteRootLogPath, appId1,
- USER_ME, SUFFIX, toDeleteTime);
- PathWithFileStatus app2 =
createPathWithFileStatusForAppId(remoteRootLogPath, appId2,
- USER_ME, SUFFIX, toDeleteTime);
- PathWithFileStatus app3 =
createPathWithFileStatusForAppId(remoteRootLogPath, appId3,
- USER_ME, SUFFIX, toDeleteTime);
- PathWithFileStatus app4 =
createPathWithFileStatusForAppId(remoteRootLogPath, appId4,
- USER_ME, SUFFIX, toDeleteTime);
-
- when(mockFs.listStatus(userDir.path)).thenReturn(new FileStatus[]
{suffixDir.fileStatus});
- when(mockFs.listStatus(suffixDir.path)).thenReturn(new FileStatus[]
{bucketDir.fileStatus});
- when(mockFs.listStatus(bucketDir.path)).thenReturn(new FileStatus[] {
- app1.fileStatus, app2.fileStatus, app3.fileStatus, app4.fileStatus});
-
- PathWithFileStatus app2Log1 = createFileLogPathWithFileStatus(app2.path,
DIR_HOST1,
- toDeleteTime);
- PathWithFileStatus app2Log2 = createFileLogPathWithFileStatus(app2.path,
DIR_HOST2, toKeepTime);
- PathWithFileStatus app3Log1 = createFileLogPathWithFileStatus(app3.path,
DIR_HOST1,
- toDeleteTime);
- PathWithFileStatus app3Log2 = createFileLogPathWithFileStatus(app3.path,
DIR_HOST2,
- toDeleteTime);
- PathWithFileStatus app4Log1 = createFileLogPathWithFileStatus(app4.path,
DIR_HOST1,
- toDeleteTime);
- PathWithFileStatus app4Log2 = createFileLogPathWithFileStatus(app4.path,
DIR_HOST2, toKeepTime);
-
- when(mockFs.listStatus(app1.path)).thenReturn(new FileStatus[]{});
- when(mockFs.listStatus(app2.path)).thenReturn(new
FileStatus[]{app2Log1.fileStatus,
- app2Log2.fileStatus});
- when(mockFs.listStatus(app3.path)).thenReturn(new
FileStatus[]{app3Log1.fileStatus,
- app3Log2.fileStatus});
- when(mockFs.listStatus(app4.path)).thenReturn(new
FileStatus[]{app4Log1.fileStatus,
- app4Log2.fileStatus});
- when(mockFs.delete(app3.path, true)).thenThrow(
- new AccessControlException("Injected Error\nStack Trace :("));
-
- final List<ApplicationId> finishedApplications =
Collections.unmodifiableList(
- Arrays.asList(appId1, appId2, appId3));
- final List<ApplicationId> runningApplications =
Collections.singletonList(appId4);
-
- AggregatedLogDeletionService deletionService =
- new AggregatedLogDeletionServiceForTest(runningApplications,
finishedApplications);
- deletionService.init(conf);
- deletionService.start();
-
- int timeout = 2000;
- verify(mockFs, timeout(timeout)).delete(app1.path, true);
- verify(mockFs, timeout(timeout).times(0)).delete(app2.path, true);
- verify(mockFs, timeout(timeout)).delete(app3.path, true);
- verify(mockFs, timeout(timeout).times(0)).delete(app4.path, true);
- verify(mockFs, timeout(timeout)).delete(app4Log1.path, true);
- verify(mockFs, timeout(timeout).times(0)).delete(app4Log2.path, true);
-
- deletionService.stop();
+ long timeout = 2000L;
+ LogAggregationFilesBuilder.create(conf)
+ .withRootPath(ROOT)
+ .withRemoteRootLogPath(REMOTE_ROOT_LOG_DIR)
+ .withUserDir(USER_ME, toKeepTime)
+ .withSuffixDir(SUFFIX, toDeleteTime)
+ .withBucketDir(toDeleteTime)
+ .withApps(new
LogAggregationFilesBuilder.AppDescriptor(toDeleteTime, new Pair[] {}),
+ new LogAggregationFilesBuilder.AppDescriptor(toDeleteTime,
+ Pair.of(DIR_HOST1, toDeleteTime),
+ Pair.of(DIR_HOST2, toKeepTime)),
+ new LogAggregationFilesBuilder.AppDescriptor(toDeleteTime,
+ Pair.of(DIR_HOST1, toDeleteTime),
+ Pair.of(DIR_HOST2, toDeleteTime)),
+ new LogAggregationFilesBuilder.AppDescriptor(toDeleteTime,
+ Pair.of(DIR_HOST1, toDeleteTime),
+ Pair.of(DIR_HOST2, toKeepTime)))
+ .withFinishedApps(1, 2, 3)
+ .withRunningApps(4)
+ .injectExceptionForAppDirDeletion(3)
+ .setupMocks()
+ .setupAndRunDeletionService()
+ .verifyAppDirsDeleted(timeout, 1, 3)
+ .verifyAppDirsNotDeleted(timeout, 2, 4)
+ .verifyAppFileDeleted(4, 1, timeout)
+ .verifyAppFileNotDeleted(4, 2, timeout)
+ .teardown();
Review Comment:
Yeah, I had the same in mind but wanted to have some feedback first. Will
change this and separate the builder from the actual mocking logic + the
verifications
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
