From 150843b26414ece3d9076c56c7d9caed63529380 Mon Sep 17 00:00:00 2001 From: maugli Date: Tue, 14 Mar 2017 12:23:35 +0100 Subject: [PATCH] SQOOP-3100: Clean up expected exception logic in tests - part III. (Boglarka Egyed via Attila Szabo) --- ivy/libraries.properties | 2 +- .../cloudera/sqoop/TestIncrementalImport.java | 17 ++-- .../cloudera/sqoop/lib/TestRecordParser.java | 48 ++++++------ .../sqoop/metastore/TestSavedJobs.java | 78 ++++++++++++++----- .../cloudera/sqoop/orm/TestClassWriter.java | 47 +++++------ 5 files changed, 114 insertions(+), 78 deletions(-) diff --git a/ivy/libraries.properties b/ivy/libraries.properties index 05baa5cb..84b834b2 100644 --- a/ivy/libraries.properties +++ b/ivy/libraries.properties @@ -36,7 +36,7 @@ hsqldb.version=1.8.0.10 ivy.version=2.3.0 -junit.version=4.11 +junit.version=4.12 mockito-all.version=1.9.5 h2.version=1.3.170 diff --git a/src/test/com/cloudera/sqoop/TestIncrementalImport.java b/src/test/com/cloudera/sqoop/TestIncrementalImport.java index 57f4433d..52a55b78 100644 --- a/src/test/com/cloudera/sqoop/TestIncrementalImport.java +++ b/src/test/com/cloudera/sqoop/TestIncrementalImport.java @@ -49,7 +49,9 @@ import com.cloudera.sqoop.tool.ImportTool; import com.cloudera.sqoop.tool.JobTool; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import static org.junit.Assert.*; @@ -70,6 +72,9 @@ public class TestIncrementalImport { // What database do we read from. public static final String SOURCE_DB_URL = "jdbc:hsqldb:mem:incremental"; + @Rule + public ExpectedException thrown = ExpectedException.none(); + @Before public void setUp() throws Exception { // Delete db state between tests. @@ -949,14 +954,12 @@ public void testAppendWithString() throws Exception { List args = getArgListForTable(TABLE_NAME, false, true); args.add("--append"); createJob(TABLE_NAME, args); - try { - runJob(TABLE_NAME); - //the above line should throw an exception otherwise the test has failed - fail("Expected incremental import on varchar column to fail."); - } catch(RuntimeException e) { - //expected - } + + thrown.expect(RuntimeException.class); + thrown.reportMissingExceptionWithMessage("Expected incremental import on varchar column to fail"); + runJob(TABLE_NAME); } + @Test public void testModifyWithTimestamp() throws Exception { // Create a table with data in it; import it. diff --git a/src/test/com/cloudera/sqoop/lib/TestRecordParser.java b/src/test/com/cloudera/sqoop/lib/TestRecordParser.java index 57bdb5fe..d964cef0 100644 --- a/src/test/com/cloudera/sqoop/lib/TestRecordParser.java +++ b/src/test/com/cloudera/sqoop/lib/TestRecordParser.java @@ -20,7 +20,10 @@ import java.util.ArrayList; import java.util.List; + +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import static org.junit.Assert.fail; @@ -30,6 +33,9 @@ */ public class TestRecordParser { + @Rule + public ExpectedException thrown = ExpectedException.none(); + private void assertListsEqual(String msg, List expected, List actual) { if (expected == null && actual != null) { @@ -300,49 +306,41 @@ public void testRequiredQuotes1() throws RecordParser.ParseError { public void testRequiredQuotes2() throws RecordParser.ParseError { RecordParser parser = new RecordParser( new DelimiterSet(',', '\n', '\"', '\\', true)); - try { - parser.parseRecord("\"field1\",field2"); - fail("Expected parse error for required quotes"); - } catch (RecordParser.ParseError pe) { - // ok. expected. - } + + thrown.expect(RecordParser.ParseError.class); + thrown.reportMissingExceptionWithMessage("Expected parse error for required quotes"); + parser.parseRecord("\"field1\",field2"); } @Test public void testRequiredQuotes3() throws RecordParser.ParseError { RecordParser parser = new RecordParser( new DelimiterSet(',', '\n', '\"', '\\', true)); - try { - parser.parseRecord("field1,\"field2\""); - fail("Expected parse error for required quotes"); - } catch (RecordParser.ParseError pe) { - // ok. expected. - } + + thrown.expect(RecordParser.ParseError.class); + thrown.reportMissingExceptionWithMessage("Expected ParseError for required quotes"); + parser.parseRecord("field1,\"field2\""); } @Test public void testRequiredQuotes4() throws RecordParser.ParseError { RecordParser parser = new RecordParser( new DelimiterSet(',', '\n', '\"', '\\', true)); - try { - parser.parseRecord("field1,\"field2\"\n"); - fail("Expected parse error for required quotes"); - } catch (RecordParser.ParseError pe) { - // ok. expected. - } + + thrown.expect(RecordParser.ParseError.class); + thrown.reportMissingExceptionWithMessage("Expected ParseError for required quotes"); + parser.parseRecord("field1,\"field2\"\n"); } @Test - public void testNull() { + public void testNull() throws RecordParser.ParseError { RecordParser parser = new RecordParser( new DelimiterSet(',', '\n', '\"', '\\', true)); String input = null; - try { - parser.parseRecord(input); - fail("Expected parse error for null string"); - } catch (RecordParser.ParseError pe) { - // ok. expected. - } + + thrown.expect(RecordParser.ParseError.class); + thrown.reportMissingExceptionWithMessage("Expected ParseError for null string"); + parser.parseRecord(input); } diff --git a/src/test/com/cloudera/sqoop/metastore/TestSavedJobs.java b/src/test/com/cloudera/sqoop/metastore/TestSavedJobs.java index 1fb73243..61d8c97d 100644 --- a/src/test/com/cloudera/sqoop/metastore/TestSavedJobs.java +++ b/src/test/com/cloudera/sqoop/metastore/TestSavedJobs.java @@ -33,13 +33,14 @@ import com.cloudera.sqoop.tool.VersionTool; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import java.io.IOException; import java.sql.Connection; import static org.junit.Assert.assertEquals; -import static org.junit.Assert.fail; /** * Test the metastore and job-handling features. @@ -55,6 +56,9 @@ public class TestSavedJobs { public static final String TEST_AUTOCONNECT_USER = "SA"; public static final String TEST_AUTOCONNECT_PASS = ""; + @Rule + public ExpectedException thrown = ExpectedException.none(); + @Before public void setUp() throws Exception { // Delete db state between tests. @@ -118,7 +122,7 @@ public void testAutoConnect() throws IOException { } @Test - public void testCreateDeleteJob() throws IOException { + public void testCreateSameJob() throws IOException { Configuration conf = newConf(); JobStorageFactory ssf = new JobStorageFactory(conf); @@ -139,29 +143,45 @@ public void testCreateDeleteJob() throws IOException { assertEquals(1, jobs.size()); assertEquals("versionJob", jobs.get(0)); - // Try to create that same job name again. This should fail. try { + // Try to create that same job name again. This should fail. + thrown.expect(IOException.class); + thrown.reportMissingExceptionWithMessage("Expected IOException since job already exists"); storage.create("versionJob", data); - fail("Expected IOException; this job already exists."); - } catch (IOException ioe) { - // This is expected; continue operation. + } finally { + jobs = storage.list(); + assertEquals(1, jobs.size()); + + // Restore our job, check that it exists. + JobData outData = storage.read("versionJob"); + assertEquals(new VersionTool().getToolName(), + outData.getSqoopTool().getToolName()); + + storage.close(); } + } + + @Test + public void testDeleteJob() throws IOException { + Configuration conf = newConf(); + JobStorageFactory ssf = new JobStorageFactory(conf); + + Map descriptor = new TreeMap(); + JobStorage storage = ssf.getJobStorage(descriptor); + + storage.open(descriptor); + + // Job list should start out empty. + List jobs = storage.list(); + assertEquals(0, jobs.size()); + + // Create a job that displays the version. + JobData data = new JobData(new SqoopOptions(), new VersionTool()); + storage.create("versionJob", data); jobs = storage.list(); assertEquals(1, jobs.size()); - - // Restore our job, check that it exists. - JobData outData = storage.read("versionJob"); - assertEquals(new VersionTool().getToolName(), - outData.getSqoopTool().getToolName()); - - // Try to restore a job that doesn't exist. Watch it fail. - try { - storage.read("DoesNotExist"); - fail("Expected IOException"); - } catch (IOException ioe) { - // This is expected. Continue. - } + assertEquals("versionJob", jobs.get(0)); // Now delete the job. storage.delete("versionJob"); @@ -173,6 +193,26 @@ public void testCreateDeleteJob() throws IOException { storage.close(); } + @Test + public void testRestoreNonExistingJob() throws IOException { + Configuration conf = newConf(); + JobStorageFactory ssf = new JobStorageFactory(conf); + + Map descriptor = new TreeMap(); + JobStorage storage = ssf.getJobStorage(descriptor); + + storage.open(descriptor); + + try { + // Try to restore a job that doesn't exist. Watch it fail. + thrown.expect(IOException.class); + thrown.reportMissingExceptionWithMessage("Expected IOException since job doesn't exist"); + storage.read("DoesNotExist"); + } finally { + storage.close(); + } + } + @Test public void testCreateJobWithExtraArgs() throws IOException { Configuration conf = newConf(); diff --git a/src/test/com/cloudera/sqoop/orm/TestClassWriter.java b/src/test/com/cloudera/sqoop/orm/TestClassWriter.java index a27353df..b3a8a174 100644 --- a/src/test/com/cloudera/sqoop/orm/TestClassWriter.java +++ b/src/test/com/cloudera/sqoop/orm/TestClassWriter.java @@ -36,6 +36,7 @@ import org.apache.hadoop.util.Shell; import org.junit.After; import org.junit.Before; +import org.junit.Rule; import org.junit.Test; import com.cloudera.sqoop.SqoopOptions; @@ -46,6 +47,7 @@ import com.cloudera.sqoop.testutil.ImportJobTestCase; import com.cloudera.sqoop.tool.ImportTool; import com.cloudera.sqoop.util.ClassLoaderStack; +import org.junit.rules.ExpectedException; import java.lang.reflect.Field; @@ -72,6 +74,9 @@ public class TestClassWriter { private ConnManager manager; private SqoopOptions options; + @Rule + public ExpectedException thrown = ExpectedException.none(); + @Before public void setUp() { testServer = new HsqldbTestServer(); @@ -642,31 +647,6 @@ public void testBrokenUserMapping() throws Exception { fail("we shouldn't successfully generate code"); } - private void runFailedGenerationTest(String [] argv, - String classNameToCheck) { - File codeGenDirFile = new File(CODE_GEN_DIR); - File classGenDirFile = new File(JAR_GEN_DIR); - - try { - options = new ImportTool().parseArguments(argv, - null, options, true); - } catch (Exception e) { - LOG.error("Could not parse options: " + e.toString()); - } - - CompilationManager compileMgr = new CompilationManager(options); - ClassWriter writer = new ClassWriter(options, manager, - HsqldbTestServer.getTableName(), compileMgr); - - try { - writer.generate(); - compileMgr.compile(); - fail("ORM class file generation succeeded when it was expected to fail"); - } catch (Exception ioe) { - LOG.error("Got Exception from ORM generation as expected : " - + ioe.toString()); - } - } /** * A dummy manager that declares that it ORM is self managed. */ @@ -686,7 +666,22 @@ public void testNoClassGeneration() throws Exception { "--outdir", CODE_GEN_DIR, }; - runFailedGenerationTest(argv, HsqldbTestServer.getTableName()); + + try { + options = new ImportTool().parseArguments(argv, + null, options, true); + } catch (Exception e) { + LOG.error("Could not parse options: " + e.toString()); + } + + CompilationManager compileMgr = new CompilationManager(options); + ClassWriter writer = new ClassWriter(options, manager, + HsqldbTestServer.getTableName(), compileMgr); + + writer.generate(); + + thrown.expect(Exception.class); + compileMgr.compile(); } @Test(timeout = 25000)