From 13c01625ad416f44c2e27a2ecb3d6abcca4f116a Mon Sep 17 00:00:00 2001 From: Abraham Elmahrek Date: Tue, 23 Dec 2014 15:07:53 -0800 Subject: [PATCH] SQOOP-1935: Sqoop2: Fix TestSqoopWritable test and make getString and setString package private (Veena Basavaraj via Abraham Elmahrek) --- .../apache/sqoop/job/io/SqoopWritable.java | 15 ++--- .../job/mr/SqoopOutputFormatLoadExecutor.java | 2 +- .../sqoop/job/io/TestSqoopWritable.java | 62 ++++++++++++------- .../apache/sqoop/job/util/MRJobTestUtil.java | 14 ++--- 4 files changed, 55 insertions(+), 38 deletions(-) diff --git a/execution/mapreduce/src/main/java/org/apache/sqoop/job/io/SqoopWritable.java b/execution/mapreduce/src/main/java/org/apache/sqoop/job/io/SqoopWritable.java index 6a0bfa47..5967f5c1 100644 --- a/execution/mapreduce/src/main/java/org/apache/sqoop/job/io/SqoopWritable.java +++ b/execution/mapreduce/src/main/java/org/apache/sqoop/job/io/SqoopWritable.java @@ -31,12 +31,16 @@ /** * Writable used to load the data to the {@link #Transferable} entity TO + * It is used for the map output key class and the outputFormat key class in the MR engine + * For instance: job.setMapOutputKeyClass(..,) and job.setOutputKeyClass(...); */ public class SqoopWritable implements Configurable, WritableComparable { private IntermediateDataFormat toIDF; private Configuration conf; + // NOTE: You have to provide an empty default constructor in your key class + // Hadoop is using reflection and it can not guess any parameters to feed public SqoopWritable() { this(null); } @@ -45,14 +49,11 @@ public SqoopWritable(IntermediateDataFormat dataFormat) { this.toIDF = dataFormat; } - public void setString(String data) { + // default/package visibility for testing + void setString(String data) { this.toIDF.setCSVTextData(data); } - public String getString() { - return toIDF.getCSVTextData(); - } - @Override public void write(DataOutput out) throws IOException { //delegate @@ -67,12 +68,12 @@ public void readFields(DataInput in) throws IOException { @Override public int compareTo(SqoopWritable o) { - return getString().compareTo(o.getString()); + return toString().compareTo(o.toString()); } @Override public String toString() { - return getString(); + return toIDF.getCSVTextData(); } @Override diff --git a/execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopOutputFormatLoadExecutor.java b/execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopOutputFormatLoadExecutor.java index 7835e38f..58a9eed6 100644 --- a/execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopOutputFormatLoadExecutor.java +++ b/execution/mapreduce/src/main/java/org/apache/sqoop/job/mr/SqoopOutputFormatLoadExecutor.java @@ -102,7 +102,7 @@ public void write(SqoopWritable key, NullWritable value) throws InterruptedExcep free.acquire(); checkIfConsumerThrew(); // NOTE: this is the place where data written from SqoopMapper writable is available to the SqoopOutputFormat - toDataFormat.setCSVTextData(key.getString()); + toDataFormat.setCSVTextData(key.toString()); filled.release(); } diff --git a/execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestSqoopWritable.java b/execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestSqoopWritable.java index b07a0763..79d6a8fa 100644 --- a/execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestSqoopWritable.java +++ b/execution/mapreduce/src/test/java/org/apache/sqoop/job/io/TestSqoopWritable.java @@ -18,6 +18,11 @@ */ package org.apache.sqoop.job.io; +import static org.junit.Assert.*; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; import java.io.DataInput; @@ -28,61 +33,72 @@ import java.io.InputStream; import org.apache.sqoop.connector.idf.CSVIntermediateDataFormat; -import org.junit.Assert; +import org.apache.sqoop.connector.idf.IntermediateDataFormat; +import org.junit.Before; import org.junit.Test; public class TestSqoopWritable { - private final SqoopWritable writable = new SqoopWritable(new CSVIntermediateDataFormat()); + private SqoopWritable writable; + private IntermediateDataFormat idfMock; + + @Before + public void setUp() { + idfMock = mock(IntermediateDataFormat.class); + writable = new SqoopWritable(idfMock); + } @Test public void testStringInStringOut() { String testData = "Live Long and prosper"; writable.setString(testData); - Assert.assertEquals(testData,writable.getString()); + verify(idfMock, times(1)).setCSVTextData(testData); + writable.toString(); + verify(idfMock, times(1)).getCSVTextData(); } @Test - public void testDataWritten() throws IOException { + public void testWrite() throws IOException { String testData = "One ring to rule them all"; - writable.setString(testData); ByteArrayOutputStream ostream = new ByteArrayOutputStream(); + ostream.write(testData.getBytes()); DataOutput out = new DataOutputStream(ostream); writable.write(out); - byte[] written = ostream.toByteArray(); - InputStream instream = new ByteArrayInputStream(written); - DataInput in = new DataInputStream(instream); - String readData = in.readUTF(); - Assert.assertEquals(testData, readData); + // verify that the idf method is called, that is all the test should test + verify(idfMock, times(1)).write(out); + ostream.close(); } @Test - public void testDataRead() throws IOException { + public void testReadFields() throws IOException { String testData = "Brandywine Bridge - 20 miles!"; - ByteArrayOutputStream ostream = new ByteArrayOutputStream(); - DataOutput out = new DataOutputStream(ostream); - out.writeUTF(testData); - InputStream instream = new ByteArrayInputStream(ostream.toByteArray()); + InputStream instream = new ByteArrayInputStream(testData.getBytes()); DataInput in = new DataInputStream(instream); writable.readFields(in); - Assert.assertEquals(testData, writable.getString()); + // verify that the idf method is called, that is all the test should test + verify(idfMock, times(1)).read(in); + instream.close(); } + // NOTE: This test is testing that the write and readFields methods work + // and not really testing anything about SqoopWritable. Have kept this test since + // it existed before. @Test - public void testWriteReadUsingStream() throws IOException { + public void testWriteAndReadFields() throws IOException { String testData = "You shall not pass"; ByteArrayOutputStream ostream = new ByteArrayOutputStream(); DataOutput out = new DataOutputStream(ostream); - writable.setString(testData); - writable.write(out); + SqoopWritable writableOne = new SqoopWritable(new CSVIntermediateDataFormat()); + writableOne.setString(testData); + writableOne.write(out); byte[] written = ostream.toByteArray(); - //Don't test what the data is, test that SqoopWritable can read it. + // Don't test what the data is, test that SqoopWritable can read it. InputStream instream = new ByteArrayInputStream(written); - SqoopWritable newWritable = new SqoopWritable(new CSVIntermediateDataFormat()); + SqoopWritable writableTwo = new SqoopWritable(new CSVIntermediateDataFormat()); DataInput in = new DataInputStream(instream); - newWritable.readFields(in); - Assert.assertEquals(testData, newWritable.getString()); + writableTwo.readFields(in); + assertEquals(writableOne.toString(), writableTwo.toString()); ostream.close(); instream.close(); } diff --git a/execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java b/execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java index 5d5359e5..d4988509 100644 --- a/execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java +++ b/execution/mapreduce/src/test/java/org/apache/sqoop/job/util/MRJobTestUtil.java @@ -44,16 +44,16 @@ public class MRJobTestUtil { @SuppressWarnings("deprecation") public static boolean runJob(Configuration conf, - Class> input, - Class> mapper, - Class> output) throws IOException, + Class> inputFormatClass, + Class> mapperClass, + Class> outputFormatClass) throws IOException, InterruptedException, ClassNotFoundException { Job job = new Job(conf); - job.setInputFormatClass(input); - job.setMapperClass(mapper); + job.setInputFormatClass(inputFormatClass); + job.setMapperClass(mapperClass); job.setMapOutputKeyClass(SqoopWritable.class); job.setMapOutputValueClass(NullWritable.class); - job.setOutputFormatClass(output); + job.setOutputFormatClass(outputFormatClass); job.setOutputKeyClass(SqoopWritable.class); job.setOutputValueClass(NullWritable.class); @@ -62,7 +62,7 @@ public static boolean runJob(Configuration conf, // Hadoop 1.0 (and 0.20) have nasty bug when job committer is not called in // LocalJobRuner if (isHadoop1()) { - callOutputCommitter(job, output); + callOutputCommitter(job, outputFormatClass); } return ret;