SQOOP-3100: Clean up expected exception logic in tests - part III.

(Boglarka Egyed via Attila Szabo)

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

src/test/com/cloudera/sqoop/TestIncrementalImport.java

@@ -49,7 +49,9 @@ import com.cloudera.sqoop.testutil.CommonArgs;
 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 class TestIncrementalImport  {
     List<String> args = getArgListForTable(TABLE_NAME, false, true);
     args.add("--append");
     createJob(TABLE_NAME, args);
-    try {
+
-      runJob(TABLE_NAME);
+    thrown.expect(RuntimeException.class);
-      //the above line should throw an exception otherwise the test has failed
+    thrown.reportMissingExceptionWithMessage("Expected incremental import on varchar column to fail");
-      fail("Expected incremental import on varchar column to fail.");
+    runJob(TABLE_NAME);
-    } catch(RuntimeException e) {
-      //expected
-    }
   }
+
   @Test
   public void testModifyWithTimestamp() throws Exception {
     // Create a table with data in it; import it.

src/test/com/cloudera/sqoop/lib/TestRecordParser.java

@@ -20,7 +20,10 @@ package com.cloudera.sqoop.lib;
 
 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 @@ import static org.junit.Assert.fail;
  */
 public class TestRecordParser {
 
+  @Rule
+  public ExpectedException thrown = ExpectedException.none();
+
   private void assertListsEqual(String msg, List<String> expected,
       List<String> actual) {
     if (expected == null && actual != null) {
@@ -300,49 +306,41 @@ public class TestRecordParser {
   public void testRequiredQuotes2() throws RecordParser.ParseError {
     RecordParser parser = new RecordParser(
         new DelimiterSet(',', '\n', '\"', '\\', true));
-    try {
+
-      parser.parseRecord("\"field1\",field2");
+    thrown.expect(RecordParser.ParseError.class);
-      fail("Expected parse error for required quotes");
+    thrown.reportMissingExceptionWithMessage("Expected parse error for required quotes");
-    } catch (RecordParser.ParseError pe) {
+    parser.parseRecord("\"field1\",field2");
-      // ok. expected.
-    }
   }
 
   @Test
   public void testRequiredQuotes3() throws RecordParser.ParseError {
     RecordParser parser = new RecordParser(
         new DelimiterSet(',', '\n', '\"', '\\', true));
-    try {
+
-      parser.parseRecord("field1,\"field2\"");
+    thrown.expect(RecordParser.ParseError.class);
-      fail("Expected parse error for required quotes");
+    thrown.reportMissingExceptionWithMessage("Expected ParseError for required quotes");
-    } catch (RecordParser.ParseError pe) {
+    parser.parseRecord("field1,\"field2\"");
-      // ok. expected.
-    }
   }
 
   @Test
   public void testRequiredQuotes4() throws RecordParser.ParseError {
     RecordParser parser = new RecordParser(
         new DelimiterSet(',', '\n', '\"', '\\', true));
-    try {
+
-      parser.parseRecord("field1,\"field2\"\n");
+    thrown.expect(RecordParser.ParseError.class);
-      fail("Expected parse error for required quotes");
+    thrown.reportMissingExceptionWithMessage("Expected ParseError for required quotes");
-    } catch (RecordParser.ParseError pe) {
+    parser.parseRecord("field1,\"field2\"\n");
-      // ok. expected.
-    }
   }
 
   @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);
+    thrown.expect(RecordParser.ParseError.class);
-      fail("Expected parse error for null string");
+    thrown.reportMissingExceptionWithMessage("Expected ParseError for null string");
-    } catch (RecordParser.ParseError pe) {
+    parser.parseRecord(input);
-      // ok. expected.
-    }
   }
 
 

src/test/com/cloudera/sqoop/metastore/TestSavedJobs.java

@@ -33,13 +33,14 @@ import com.cloudera.sqoop.metastore.hsqldb.AutoHsqldbStorage;
 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 class TestSavedJobs {
   }
 
   @Test
-  public void testCreateDeleteJob() throws IOException {
+  public void testCreateSameJob() throws IOException {
     Configuration conf = newConf();
     JobStorageFactory ssf = new JobStorageFactory(conf);
 
@@ -139,29 +143,45 @@ public class TestSavedJobs {
     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.");
+    } finally {
-    } catch (IOException ioe) {
+      jobs = storage.list();
-      // This is expected; continue operation.
+      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<String, String> descriptor = new TreeMap<String, String>();
+    JobStorage storage = ssf.getJobStorage(descriptor);
+
+    storage.open(descriptor);
+
+    // Job list should start out empty.
+    List<String> 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());
-
+    assertEquals("versionJob", jobs.get(0));
-    // 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.
-    }
 
     // Now delete the job.
     storage.delete("versionJob");
@@ -173,6 +193,26 @@ public class TestSavedJobs {
     storage.close();
   }
 
+  @Test
+  public void testRestoreNonExistingJob() throws IOException {
+    Configuration conf = newConf();
+    JobStorageFactory ssf = new JobStorageFactory(conf);
+
+    Map<String, String> descriptor = new TreeMap<String, String>();
+    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();

src/test/com/cloudera/sqoop/orm/TestClassWriter.java

@@ -36,6 +36,7 @@ import org.apache.commons.logging.LogFactory;
 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.HsqldbTestServer;
 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 class TestClassWriter {
     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 class TestClassWriter {
       "--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)