mukund-thakur commented on code in PR #6857: URL: https://github.com/apache/hadoop/pull/6857#discussion_r1628384180
########## hadoop-tools/hadoop-aws/src/test/java/org/apache/hadoop/fs/s3a/commit/ITestS3ACommitterFactory.java: ########## @@ -72,121 +85,141 @@ public class ITestS3ACommitterFactory extends AbstractCommitITest { * Parameterized list of bindings of committer name in config file to * expected class instantiated. */ - private static final Object[][] bindings = { - {COMMITTER_NAME_FILE, FileOutputCommitter.class}, - {COMMITTER_NAME_DIRECTORY, DirectoryStagingCommitter.class}, - {COMMITTER_NAME_PARTITIONED, PartitionedStagingCommitter.class}, - {InternalCommitterConstants.COMMITTER_NAME_STAGING, - StagingCommitter.class}, - {COMMITTER_NAME_MAGIC, MagicS3GuardCommitter.class} + private static final Object[][] BINDINGS = { + {"", "", FileOutputCommitter.class, "Default Binding"}, + {COMMITTER_NAME_FILE, "", FileOutputCommitter.class, "File committer in FS"}, + {COMMITTER_NAME_PARTITIONED, "", PartitionedStagingCommitter.class, + "partitoned committer in FS"}, + {COMMITTER_NAME_STAGING, "", StagingCommitter.class, "staging committer in FS"}, + {COMMITTER_NAME_MAGIC, "", MagicS3GuardCommitter.class, "magic committer in FS"}, + {COMMITTER_NAME_DIRECTORY, "", DirectoryStagingCommitter.class, "Dir committer in FS"}, + {INVALID_NAME, "", null, "invalid committer in FS"}, + + {"", COMMITTER_NAME_FILE, FileOutputCommitter.class, "File committer in task"}, + {"", COMMITTER_NAME_PARTITIONED, PartitionedStagingCommitter.class, + "partioned committer in task"}, + {"", COMMITTER_NAME_STAGING, StagingCommitter.class, "staging committer in task"}, + {"", COMMITTER_NAME_MAGIC, MagicS3GuardCommitter.class, "magic committer in task"}, + {"", COMMITTER_NAME_DIRECTORY, DirectoryStagingCommitter.class, "Dir committer in task"}, + {"", INVALID_NAME, null, "invalid committer in task"}, }; /** - * This is a ref to the FS conf, so changes here are visible - * to callers querying the FS config. + * Test array for parameterized test runs. + * + * @return the committer binding for this run. */ - private Configuration filesystemConfRef; - - private Configuration taskConfRef; - - @Override - public void setup() throws Exception { - super.setup(); - jobId = randomJobId(); - attempt0 = "attempt_" + jobId + "_m_000000_0"; - taskAttempt0 = TaskAttemptID.forName(attempt0); - - outDir = path(getMethodName()); - factory = new S3ACommitterFactory(); - Configuration conf = new Configuration(); - conf.set(FileOutputFormat.OUTDIR, outDir.toUri().toString()); - conf.set(MRJobConfig.TASK_ATTEMPT_ID, attempt0); - conf.setInt(MRJobConfig.APPLICATION_ATTEMPT_ID, 1); - filesystemConfRef = getFileSystem().getConf(); - tContext = new TaskAttemptContextImpl(conf, taskAttempt0); - taskConfRef = tContext.getConfiguration(); - } - - @Test - public void testEverything() throws Throwable { - testImplicitFileBinding(); - testBindingsInTask(); - testBindingsInFSConfig(); - testInvalidFileBinding(); - testInvalidTaskBinding(); + @Parameterized.Parameters(name = "{3}-fs=[{0}]-task=[{1}]-[{2}]") + public static Collection<Object[]> params() { + return Arrays.asList(BINDINGS); } /** - * Verify that if all config options are unset, the FileOutputCommitter - * - * is returned. + * Name of committer to set in fs config. If "" do not set one. */ - public void testImplicitFileBinding() throws Throwable { - taskConfRef.unset(FS_S3A_COMMITTER_NAME); - filesystemConfRef.unset(FS_S3A_COMMITTER_NAME); - assertFactoryCreatesExpectedCommitter(FileOutputCommitter.class); - } + private final String fsCommitterName; + private final String pathCommitterName; Review Comment: why do we need this? Isn't just first one enough -- 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: common-issues-unsubscr...@hadoop.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: common-issues-unsubscr...@hadoop.apache.org For additional commands, e-mail: common-issues-h...@hadoop.apache.org