abstractdog commented on code in PR #266:
URL: https://github.com/apache/tez/pull/266#discussion_r1116686434
##########
tez-dag/src/test/java/org/apache/tez/dag/app/TestDAGAppMaster.java:
##########
@@ -332,6 +339,92 @@ public void testParseAllPluginsCustomAndYarnSpecified()
throws IOException {
assertEquals(TC_NAME + CLASS_SUFFIX, tcDescriptors.get(1).getClassName());
}
+ @Test
+ public void testShutdownTezAMWithMissingRecovery() throws Exception {
+
+ TezConfiguration conf = new TezConfiguration();
+ conf.setBoolean(TezConfiguration.TEZ_AM_CREDENTIALS_MERGE, true);
+ conf.setBoolean(TezConfiguration.TEZ_LOCAL_MODE, true);
+ conf.set(TezConfiguration.TEZ_AM_STAGING_DIR, TEST_DIR.toString());
+ conf.setBoolean(TezConfiguration.TEZ_AM_FAILURE_ON_MISSING_RECOVERY_DATA,
true);
+ conf.setBoolean(TezConfiguration.DAG_RECOVERY_ENABLED, true);
+ ApplicationId appId = ApplicationId.newInstance(1, 1);
+ ApplicationAttemptId attemptId = ApplicationAttemptId.newInstance(appId,
2);
+
+ FileSystem spyRecoveryFs = spy(new FileSystem() {
+ @Override
+ public URI getUri() {
+ return null;
+ }
+
+ @Override
+ public FSDataInputStream open(Path path, int i) throws IOException {
+ return null;
+ }
+
+ @Override
+ public FSDataOutputStream create(Path path, FsPermission fsPermission,
boolean b,
+ int i, short i1, long l, Progressable
progressable)
+ throws IOException {
+ return null;
+ }
+
+ @Override
+ public FSDataOutputStream append(Path path, int i, Progressable
progressable) throws IOException {
+ return null;
+ }
+
+ @Override
+ public boolean rename(Path path, Path path1) throws IOException {
+ return false;
+ }
+
+ @Override
+ public boolean delete(Path path, boolean b) throws IOException {
+ return false;
+ }
+
+ @Override
+ public FileStatus[] listStatus(Path path) throws FileNotFoundException,
IOException {
+ return new FileStatus[0];
+ }
+
+ @Override
+ public void setWorkingDirectory(Path path) {
+
+ }
+
+ @Override
+ public Path getWorkingDirectory() {
+ return null;
+ }
+
+ @Override
+ public boolean mkdirs(Path path, FsPermission fsPermission) throws
IOException {
+ return false;
+ }
+
+ @Override
+ public FileStatus getFileStatus(Path path) throws IOException {
+ return null;
+ }
+ });
+ when(spyRecoveryFs.exists(any())).thenReturn(false);
+
+ DAGAppMasterForTest dam = new DAGAppMasterForTest(attemptId, true);
+ dam.init(conf);
+ dam.start();
+
+ Field field =
DAGAppMasterForTest.class.getSuperclass().getDeclaredField("recoveryFS");
+ field.setAccessible(true);
+ field.set(dam, spyRecoveryFs);
+
+ verify(dam.mockScheduler).setShouldUnregisterFlag();
+ verify(dam.mockShutdown).shutdown();
Review Comment:
thanks @mudit-97 , just a few questions:
1. I'm not 100% sure, but do we really need spy here? as far as I know we
use spies when really need an instance created by us instead of a mocked one,
would you consider trying to use a mock here, which might be much simpler?
2. if we end up using a spy here, please fix the method arguments' name...I
know, this might look nitpicking, but even if we don't use them now, it's
always a code smell to have args like "boolean b, int i, short i1, long l"
--
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]
