Apply patch from bug 58909 - Add a cloneSheet() which directly sets the sheetname to allow to avoid a costly renaming of sheets.

Combine related unit-tests into Base-Test-Classes to run them for all types of Workbook/Sheet/...

git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1737237 13f79535-47bb-0310-9956-ffa450edef68

6 files changed, 230 insertions, 156 deletions

diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestCloneSheet.java b/src/testcases/org/apache/poi/hssf/usermodel/TestCloneSheet.java
index 3bcd66bd47..5bdfe6227b 100644
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestCloneSheet.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestCloneSheet.java
@@ -17,18 +17,18 @@
 
 package org.apache.poi.hssf.usermodel;
 
-import static org.junit.Assert.assertArrayEquals;
-
-import java.io.IOException;
-import java.util.Arrays;
-
-import junit.framework.TestCase;
-
 import org.apache.poi.ddf.EscherDgRecord;
 import org.apache.poi.ddf.EscherSpRecord;
+import org.apache.poi.hssf.HSSFITestDataProvider;
 import org.apache.poi.hssf.record.CommonObjectDataSubRecord;
 import org.apache.poi.hssf.record.EscherAggregate;
-import org.apache.poi.ss.util.CellRangeAddress;
+import org.apache.poi.ss.usermodel.BaseTestCloneSheet;
+import org.junit.Test;
+
+import java.io.IOException;
+import java.util.Arrays;
+
+import static org.junit.Assert.*;
 
 /**
  * Test the ability to clone a sheet.
@@ -36,40 +36,12 @@ import org.apache.poi.ss.util.CellRangeAddress;
  *  add that record to the sheet in the testCloneSheetBasic method.
  * @author  avik
  */
-public final class TestCloneSheet extends TestCase {
-
-	public void testCloneSheetBasic() throws IOException{
-		HSSFWorkbook b = new HSSFWorkbook();
-		HSSFSheet s = b.createSheet("Test");
-		s.addMergedRegion(new CellRangeAddress(0, 1, 0, 1));
-		HSSFSheet clonedSheet = b.cloneSheet(0);
-
-		assertEquals("One merged area", 1, clonedSheet.getNumMergedRegions());
-		
-		b.close();
-	}
-
-	/**
-	 * Ensures that pagebreak cloning works properly
-	 * @throws IOException 
-	 */
-	public void testPageBreakClones() throws IOException {
-		HSSFWorkbook b = new HSSFWorkbook();
-		HSSFSheet s = b.createSheet("Test");
-		s.setRowBreak(3);
-		s.setColumnBreak((short) 6);
-
-		HSSFSheet clone = b.cloneSheet(0);
-		assertTrue("Row 3 not broken", clone.isRowBroken(3));
-		assertTrue("Column 6 not broken", clone.isColumnBroken((short) 6));
-
-		s.removeRowBreak(3);
-
-		assertTrue("Row 3 still should be broken", clone.isRowBroken(3));
-		
-		b.close();
-	}
-    
+public final class TestCloneSheet extends BaseTestCloneSheet {
+    public TestCloneSheet() {
+        super(HSSFITestDataProvider.instance);
+    }
+
+    @Test
     public void testCloneSheetWithoutDrawings(){
         HSSFWorkbook b = new HSSFWorkbook();
         HSSFSheet s = b.createSheet("Test");
@@ -79,7 +51,8 @@ public final class TestCloneSheet extends TestCase {
         assertNull(s2.getDrawingPatriarch());
         assertEquals(HSSFTestHelper.getSheetForTest(s).getRecords().size(), HSSFTestHelper.getSheetForTest(s2).getRecords().size());
     }
-    
+
+    @Test
     public void testCloneSheetWithEmptyDrawingAggregate(){
         HSSFWorkbook b = new HSSFWorkbook();
         HSSFSheet s = b.createSheet("Test");
@@ -114,7 +87,8 @@ public final class TestCloneSheet extends TestCase {
         assertEquals(agg1.toXml(""), agg2.toXml(""));
         assertArrayEquals(agg1.serialize(), agg2.serialize());
     }
-    
+
+    @Test
     public void testCloneComment() throws IOException {
         HSSFWorkbook wb = new HSSFWorkbook();
         HSSFSheet sh = wb.createSheet();
diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java
index 56fdc2ef2c..ed43b4264c 100644
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFSheet.java
@@ -964,7 +964,7 @@ public final class TestHSSFSheet extends BaseTestSheet {
             wb.createSheet(SAME_PREFIX + "Dyyyy"); // identical up to the 32nd char
             fail("Expected exception not thrown");
         } catch (IllegalArgumentException e) {
-            assertEquals("The workbook already contains a sheet of this name", e.getMessage());
+            assertEquals("The workbook already contains a sheet named 'A123456789B123456789C123456789Dyyyy'", e.getMessage());
         }
         wb.createSheet(SAME_PREFIX + "Exxxx"); // OK - differs in the 31st char
         wb.close();
diff --git a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
index a5f48eec2e..167e41adf6 100644
--- a/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
+++ b/src/testcases/org/apache/poi/hssf/usermodel/TestHSSFWorkbook.java
@@ -48,7 +48,6 @@ import org.apache.poi.hssf.record.CFRuleRecord;
 import org.apache.poi.hssf.record.NameRecord;
 import org.apache.poi.hssf.record.Record;
 import org.apache.poi.hssf.record.RecordBase;
-import org.apache.poi.hssf.record.RecordFormatException;
 import org.apache.poi.hssf.record.WindowOneRecord;
 import org.apache.poi.poifs.filesystem.DirectoryEntry;
 import org.apache.poi.poifs.filesystem.DirectoryNode;
@@ -56,13 +55,10 @@ import org.apache.poi.poifs.filesystem.NPOIFSFileSystem;
 import org.apache.poi.poifs.filesystem.POIFSFileSystem;
 import org.apache.poi.ss.SpreadsheetVersion;
 import org.apache.poi.ss.formula.ptg.Area3DPtg;
-import org.apache.poi.ss.usermodel.BaseTestWorkbook;
-import org.apache.poi.ss.usermodel.Name;
-import org.apache.poi.ss.usermodel.Row;
-import org.apache.poi.ss.usermodel.Sheet;
-import org.apache.poi.ss.usermodel.Workbook;
+import org.apache.poi.ss.usermodel.*;
 import org.apache.poi.ss.util.CellRangeAddress;
 import org.apache.poi.util.LittleEndian;
+import org.apache.poi.util.RecordFormatException;
 import org.apache.poi.util.TempFile;
 import org.junit.Test;
 
@@ -87,7 +83,7 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
 
     /**
      * Tests for {@link HSSFWorkbook#isHidden()} etc
-     * @throws IOException 
+     * @throws IOException
      */
     @Test
     public void hidden() throws IOException {
@@ -112,36 +108,14 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         wbBack.setHidden(false);
         assertEquals(false, wbBack.isHidden());
         assertEquals(false, w1.getHidden());
-        
+
         wbBack.close();
         wb.close();
     }
 
     @Test
-    public void sheetClone() throws IOException {
-        // First up, try a simple file
-        HSSFWorkbook b = new HSSFWorkbook();
-        assertEquals(0, b.getNumberOfSheets());
-        b.createSheet("Sheet One");
-        b.createSheet("Sheet Two");
-
-        assertEquals(2, b.getNumberOfSheets());
-        b.cloneSheet(0);
-        assertEquals(3, b.getNumberOfSheets());
-
-        // Now try a problem one with drawing records in it
-        HSSFWorkbook bBack = HSSFTestDataSamples.openSampleWorkbook("SheetWithDrawing.xls");
-        assertEquals(1, bBack.getNumberOfSheets());
-        bBack.cloneSheet(0);
-        assertEquals(2, bBack.getNumberOfSheets());
-        
-        bBack.close();
-        b.close();
-    }
-
-    @Test
     public void readWriteWithCharts() throws IOException {
-        HSSFSheet s;
+        Sheet s;
 
         // Single chart, two sheets
         HSSFWorkbook b1 = HSSFTestDataSamples.openSampleWorkbook("44010-SingleChart.xls");
@@ -196,7 +170,7 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         // So, start again
         HSSFWorkbook b5 = HSSFTestDataSamples.openSampleWorkbook("44010-TwoCharts.xls");
 
-        HSSFWorkbook b6 = HSSFTestDataSamples.writeOutAndReadBack(b5);
+        Workbook b6 = HSSFTestDataSamples.writeOutAndReadBack(b5);
         b5.close();
         assertEquals(3, b6.getNumberOfSheets());
 
@@ -281,7 +255,7 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         assertEquals(false, sheet1.isActive());
         assertEquals(true, sheet3.isActive());
 
-        if (false) { // helpful if viewing this workbook in excel:
+        /*{ // helpful if viewing this workbook in excel:
             sheet1.createRow(0).createCell(0).setCellValue(new HSSFRichTextString("Sheet1"));
             sheet2.createRow(0).createCell(0).setCellValue(new HSSFRichTextString("Sheet2"));
             sheet3.createRow(0).createCell(0).setCellValue(new HSSFRichTextString("Sheet3"));
@@ -295,7 +269,7 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
             } catch (IOException e) {
                 throw new RuntimeException(e);
             }
-        }
+        }*/
         
         wb.close();
     }
@@ -674,18 +648,17 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
                   HSSFWorkbook hw = new HSSFWorkbook(root, true);
                   List<HSSFObjectData> objects = hw.getAllEmbeddedObjects();
                   boolean found = false;
-                  for (int i = 0; i < objects.size(); i++) {
-                     HSSFObjectData embeddedObject = objects.get(i);
-                     if (embeddedObject.hasDirectoryEntry()) {
-                        DirectoryEntry dir = embeddedObject.getDirectory();
-                        if (dir instanceof DirectoryNode) {
-                           DirectoryNode dNode = (DirectoryNode)dir;
-                           if (hasEntry(dNode,"WordDocument")) {
-                              found = true;
+                   for (HSSFObjectData embeddedObject : objects) {
+                       if (embeddedObject.hasDirectoryEntry()) {
+                           DirectoryEntry dir = embeddedObject.getDirectory();
+                           if (dir instanceof DirectoryNode) {
+                               DirectoryNode dNode = (DirectoryNode) dir;
+                               if (hasEntry(dNode, "WordDocument")) {
+                                   found = true;
+                               }
                            }
-                        }
-                     }
-                  }
+                       }
+                   }
                   assertTrue(found);
                   
                   hw.close();
@@ -731,7 +704,7 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
 
     @Test
     public void cellStylesLimit() throws IOException {
-        HSSFWorkbook wb = new HSSFWorkbook();
+        Workbook wb = new HSSFWorkbook();
         int numBuiltInStyles = wb.getNumCellStyles();
         int MAX_STYLES = 4030;
         int limit = MAX_STYLES - numBuiltInStyles;
@@ -754,38 +727,38 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
 
     @Test
     public void setSheetOrderHSSF() throws IOException{
-        HSSFWorkbook wb = new HSSFWorkbook();
-        HSSFSheet s1 = wb.createSheet("first sheet");
-        HSSFSheet s2 = wb.createSheet("other sheet");
+        Workbook wb = new HSSFWorkbook();
+        Sheet s1 = wb.createSheet("first sheet");
+        Sheet s2 = wb.createSheet("other sheet");
 
-        HSSFName name1 = wb.createName();
+        Name name1 = wb.createName();
         name1.setNameName("name1");
         name1.setRefersToFormula("'first sheet'!D1");
 
-        HSSFName name2 = wb.createName();
+        Name name2 = wb.createName();
         name2.setNameName("name2");
         name2.setRefersToFormula("'other sheet'!C1");
 
 
-        HSSFRow s1r1 = s1.createRow(2);
-        HSSFCell c1 = s1r1.createCell(3);
+        Row s1r1 = s1.createRow(2);
+        Cell c1 = s1r1.createCell(3);
         c1.setCellValue(30);
-        HSSFCell c2 = s1r1.createCell(2);
+        Cell c2 = s1r1.createCell(2);
         c2.setCellFormula("SUM('other sheet'!C1,'first sheet'!C1)");
 
-        HSSFRow s2r1 = s2.createRow(0);
-        HSSFCell c3 = s2r1.createCell(1);
+        Row s2r1 = s2.createRow(0);
+        Cell c3 = s2r1.createCell(1);
         c3.setCellFormula("'first sheet'!D3");
-        HSSFCell c4 = s2r1.createCell(2);
+        Cell c4 = s2r1.createCell(2);
         c4.setCellFormula("'other sheet'!D3");
 
         // conditional formatting
-        HSSFSheetConditionalFormatting sheetCF = s1.getSheetConditionalFormatting();
+        SheetConditionalFormatting sheetCF = s1.getSheetConditionalFormatting();
 
-        HSSFConditionalFormattingRule rule1 = sheetCF.createConditionalFormattingRule(
+        ConditionalFormattingRule rule1 = sheetCF.createConditionalFormattingRule(
                 CFRuleRecord.ComparisonOperator.BETWEEN, "'first sheet'!D1", "'other sheet'!D1");
 
-        HSSFConditionalFormattingRule [] cfRules = { rule1 };
+        ConditionalFormattingRule [] cfRules = { rule1 };
 
         CellRangeAddress[] regions = {
             new CellRangeAddress(2, 4, 0, 0), // A3:A5
@@ -804,10 +777,10 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         assertEquals("'other sheet'!D3", c4.getCellFormula());
 
         // conditional formatting
-        HSSFConditionalFormatting cf = sheetCF.getConditionalFormattingAt(0);
+        ConditionalFormatting cf = sheetCF.getConditionalFormattingAt(0);
         assertEquals("'first sheet'!D1", cf.getRule(0).getFormula1());
         assertEquals("'other sheet'!D1", cf.getRule(0).getFormula2());
-        
+
         wb.close();
     }
 
@@ -901,47 +874,6 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
         wb.close();
     }
 
-    @Test
-    public void addSheetTwice() throws IOException {
-        HSSFWorkbook wb=new HSSFWorkbook();
-        HSSFSheet sheet1 = wb.createSheet("Sheet1");
-        assertNotNull(sheet1);
-        try {
-            wb.createSheet("Sheet1");
-            fail("Should fail if we add the same sheet twice");
-        } catch (IllegalArgumentException e) {
-            assertTrue(e.getMessage(), e.getMessage().contains("already contains a sheet of this name"));
-        }
-        
-        wb.close();
-    }
-
-    @Test
-    public void getSheetIndex() throws IOException {
-        HSSFWorkbook wb=new HSSFWorkbook();
-        HSSFSheet sheet1 = wb.createSheet("Sheet1");
-        HSSFSheet sheet2 = wb.createSheet("Sheet2");
-        HSSFSheet sheet3 = wb.createSheet("Sheet3");
-        HSSFSheet sheet4 = wb.createSheet("Sheet4");
-
-        assertEquals(0, wb.getSheetIndex(sheet1));
-        assertEquals(1, wb.getSheetIndex(sheet2));
-        assertEquals(2, wb.getSheetIndex(sheet3));
-        assertEquals(3, wb.getSheetIndex(sheet4));
-
-        // remove sheets
-        wb.removeSheetAt(0);
-        wb.removeSheetAt(2);
-
-        // ensure that sheets are moved up and removed sheets are not found any more
-        assertEquals(-1, wb.getSheetIndex(sheet1));
-        assertEquals(0, wb.getSheetIndex(sheet2));
-        assertEquals(1, wb.getSheetIndex(sheet3));
-        assertEquals(-1, wb.getSheetIndex(sheet4));
-        
-        wb.close();
-    }
-
     @SuppressWarnings("deprecation")
     @Test
     public void getExternSheetIndex() throws IOException {
@@ -1143,24 +1075,26 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
 		n.setRefersToFormula(sheetName + "!A1");
 
 		assertSheetOrder(wb, "Sheet1", "Sheet2", "Sheet3", "ASheet");
-		assertEquals("ASheet!A1", wb.getName(nameName).getRefersToFormula());
+        final HSSFName name = wb.getName(nameName);
+        assertNotNull(name);
+        assertEquals("ASheet!A1", name.getRefersToFormula());
 		
 		ByteArrayOutputStream stream = new ByteArrayOutputStream();
 		wb.write(stream);
 
 		assertSheetOrder(wb, "Sheet1", "Sheet2", "Sheet3", "ASheet");
-		assertEquals("ASheet!A1", wb.getName(nameName).getRefersToFormula());
+		assertEquals("ASheet!A1", name.getRefersToFormula());
 
 		wb.removeSheetAt(1);
 
 		assertSheetOrder(wb, "Sheet1", "Sheet3", "ASheet");
-		assertEquals("ASheet!A1", wb.getName(nameName).getRefersToFormula());
+		assertEquals("ASheet!A1", name.getRefersToFormula());
 
 		ByteArrayOutputStream stream2 = new ByteArrayOutputStream();
 		wb.write(stream2);
 
 		assertSheetOrder(wb, "Sheet1", "Sheet3", "ASheet");
-		assertEquals("ASheet!A1", wb.getName(nameName).getRefersToFormula());
+		assertEquals("ASheet!A1", name.getRefersToFormula());
 
 		HSSFWorkbook wb2 = new HSSFWorkbook(new ByteArrayInputStream(stream.toByteArray()));
 		expectName(wb2, nameName, "ASheet!A1");
@@ -1172,7 +1106,9 @@ public final class TestHSSFWorkbook extends BaseTestWorkbook {
 	}
 
 	private void expectName(HSSFWorkbook wb, String name, String expect) {
-		assertEquals(expect, wb.getName(name).getRefersToFormula());
+        final HSSFName hssfName = wb.getName(name);
+        assertNotNull(hssfName);
+        assertEquals(expect, hssfName.getRefersToFormula());
 	}
 
 	@Test
diff --git a/src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java b/src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java
index 321968d09c..0043187c05 100644
--- a/src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java
+++ b/src/testcases/org/apache/poi/ss/formula/BaseTestExternalFunctions.java
@@ -38,7 +38,7 @@ import org.junit.Test;
 /**
  * Test setting / evaluating of Analysis Toolpack and user-defined functions
  */
-public class BaseTestExternalFunctions {
+public abstract class BaseTestExternalFunctions {
     // define two custom user-defined functions
     private static class MyFunc implements FreeRefFunction {
         public MyFunc() {
diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestCloneSheet.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestCloneSheet.java
new file mode 100644
index 0000000000..f6c47af84d
--- /dev/null
+++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestCloneSheet.java
@@ -0,0 +1,100 @@
+/* ====================================================================
+   Licensed to the Apache Software Foundation (ASF) under one or more
+   contributor license agreements.  See the NOTICE file distributed with
+   this work for additional information regarding copyright ownership.
+   The ASF licenses this file to You under the Apache License, Version 2.0
+   (the "License"); you may not use this file except in compliance with
+   the License.  You may obtain a copy of the License at
+
+       http://www.apache.org/licenses/LICENSE-2.0
+
+   Unless required by applicable law or agreed to in writing, software
+   distributed under the License is distributed on an "AS IS" BASIS,
+   WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+   See the License for the specific language governing permissions and
+   limitations under the License.
+==================================================================== */
+
+package org.apache.poi.ss.usermodel;
+
+import org.apache.poi.ss.ITestDataProvider;
+import org.apache.poi.ss.util.CellRangeAddress;
+import org.junit.Test;
+
+import java.io.IOException;
+
+import static org.junit.Assert.*;
+
+/**
+ * Common superclass for testing implementations of
+ * Workbook.cloneSheet()
+ */
+public abstract class BaseTestCloneSheet {
+
+    private final ITestDataProvider _testDataProvider;
+
+    protected BaseTestCloneSheet(ITestDataProvider testDataProvider) {
+        _testDataProvider = testDataProvider;
+    }
+
+    @Test
+    public void testCloneSheetBasic() throws IOException{
+        Workbook b = _testDataProvider.createWorkbook();
+        Sheet s = b.createSheet("Test");
+        s.addMergedRegion(new CellRangeAddress(0, 1, 0, 1));
+        Sheet clonedSheet = b.cloneSheet(0);
+
+        assertEquals("One merged area", 1, clonedSheet.getNumMergedRegions());
+
+        b.close();
+    }
+
+    /**
+     * Ensures that pagebreak cloning works properly
+     * @throws IOException
+     */
+    @Test
+    public void testPageBreakClones() throws IOException {
+        Workbook b = _testDataProvider.createWorkbook();
+        Sheet s = b.createSheet("Test");
+        s.setRowBreak(3);
+        s.setColumnBreak((short) 6);
+
+        Sheet clone = b.cloneSheet(0);
+        assertTrue("Row 3 not broken", clone.isRowBroken(3));
+        assertTrue("Column 6 not broken", clone.isColumnBroken((short) 6));
+
+        s.removeRowBreak(3);
+
+        assertTrue("Row 3 still should be broken", clone.isRowBroken(3));
+
+        b.close();
+    }
+
+    @Test
+    public void testCloneSheetIntValid() {
+        Workbook wb = _testDataProvider.createWorkbook();
+        wb.createSheet("Sheet01");
+        wb.cloneSheet(0);
+        assertEquals(2, wb.getNumberOfSheets());
+        try {
+            wb.cloneSheet(2);
+            fail("ShouldFail");
+        } catch (IllegalArgumentException e) {
+            // expected here
+        }
+    }
+
+    @Test
+    public void testCloneSheetIntInvalid() {
+        Workbook wb = _testDataProvider.createWorkbook();
+        wb.createSheet("Sheet01");
+        try {
+            wb.cloneSheet(1);
+            fail("Should Fail");
+        } catch (IllegalArgumentException e) {
+            // expected here
+        }
+        assertEquals(1, wb.getNumberOfSheets());
+    }
+}
diff --git a/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java b/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
index 346340082a..2169db3d59 100644
--- a/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
+++ b/src/testcases/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
@@ -161,7 +161,7 @@ public abstract class BaseTestWorkbook {
             fail("should have thrown exceptiuon due to duplicate sheet name");
         } catch (IllegalArgumentException e) {
             // expected during successful test
-            assertEquals("The workbook already contains a sheet of this name", e.getMessage());
+            assertEquals("The workbook already contains a sheet named 'sHeeT3'", e.getMessage());
         }
 
         //names cannot be blank or contain any of /\*?[]
@@ -255,7 +255,7 @@ public abstract class BaseTestWorkbook {
             fail("expected exception");
         } catch (IllegalArgumentException e) {
             // expected during successful test
-            assertEquals("The workbook already contains a sheet of this name", e.getMessage());
+            assertEquals("The workbook already contains a sheet named 'My very long sheet name which is longer than 31 chars and sheetName2.substring(0, 31) == sheetName1.substring(0, 31)'", e.getMessage());
         }
 
         String sheetName3 = "POI allows creating sheets with names longer than 31 characters";
@@ -843,4 +843,68 @@ public abstract class BaseTestWorkbook {
         assertArrayEquals(filename + " sample file was modified as a result of closing the workbook",
                 before, after);
     }
+
+    @Test
+    public void sheetClone() throws IOException {
+        // First up, try a simple file
+        final Workbook b = _testDataProvider.createWorkbook();
+        assertEquals(0, b.getNumberOfSheets());
+        b.createSheet("Sheet One");
+        b.createSheet("Sheet Two");
+
+        assertEquals(2, b.getNumberOfSheets());
+        b.cloneSheet(0);
+        assertEquals(3, b.getNumberOfSheets());
+
+        // Now try a problem one with drawing records in it
+        Workbook bBack = HSSFTestDataSamples.openSampleWorkbook("SheetWithDrawing.xls");
+        assertEquals(1, bBack.getNumberOfSheets());
+        bBack.cloneSheet(0);
+        assertEquals(2, bBack.getNumberOfSheets());
+
+        bBack.close();
+        b.close();
+    }
+
+    @Test
+    public void getSheetIndex() throws IOException {
+        final Workbook wb = _testDataProvider.createWorkbook();
+        Sheet sheet1 = wb.createSheet("Sheet1");
+        Sheet sheet2 = wb.createSheet("Sheet2");
+        Sheet sheet3 = wb.createSheet("Sheet3");
+        Sheet sheet4 = wb.createSheet("Sheet4");
+
+        assertEquals(0, wb.getSheetIndex(sheet1));
+        assertEquals(1, wb.getSheetIndex(sheet2));
+        assertEquals(2, wb.getSheetIndex(sheet3));
+        assertEquals(3, wb.getSheetIndex(sheet4));
+
+        // remove sheets
+        wb.removeSheetAt(0);
+        wb.removeSheetAt(2);
+
+        // ensure that sheets are moved up and removed sheets are not found any more
+        assertEquals(-1, wb.getSheetIndex(sheet1));
+        assertEquals(0, wb.getSheetIndex(sheet2));
+        assertEquals(1, wb.getSheetIndex(sheet3));
+        assertEquals(-1, wb.getSheetIndex(sheet4));
+
+        wb.close();
+    }
+
+    @Test
+    public void addSheetTwice() throws IOException {
+        final Workbook wb = _testDataProvider.createWorkbook();
+        Sheet sheet1 = wb.createSheet("Sheet1");
+        assertNotNull(sheet1);
+        try {
+            wb.createSheet("Sheet1");
+            fail("Should fail if we add the same sheet twice");
+        } catch (IllegalArgumentException e) {
+            assertTrue(e.getMessage(), e.getMessage().contains("already contains a sheet named 'Sheet1'"));
+        }
+
+        wb.close();
+    }
+
 }