From f71cebcce5ed809ee15cd69524f8cb0b0b2ea47c Mon Sep 17 00:00:00 2001 From: Andreas Beeker Date: Sat, 28 Aug 2021 23:48:48 +0000 Subject: sonar fixes close resources in tests fix gradle warnings git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1892683 13f79535-47bb-0310-9956-ffa450edef68 --- poi/src/main/java/org/apache/poi/hpsf/ClassID.java | 17 +- .../main/java/org/apache/poi/hpsf/PropertySet.java | 83 +- poi/src/main/java/org/apache/poi/hpsf/Section.java | 75 +- .../apache/poi/hssf/record/RecordInputStream.java | 19 +- .../usermodel/HSSFConditionalFormattingRule.java | 40 +- .../HSSFConditionalFormattingThreshold.java | 12 +- .../usermodel/HSSFIconMultiStateFormatting.java | 17 +- .../poi/hssf/usermodel/HSSFWorkbookFactory.java | 8 +- .../poifs/crypt/cryptoapi/CryptoAPIDecryptor.java | 12 +- .../apache/poi/poifs/macros/VBAMacroReader.java | 62 +- .../apache/poi/ss/extractor/EmbeddedExtractor.java | 7 +- .../usermodel/ConditionalFormattingThreshold.java | 38 +- .../apache/poi/util/GenericRecordJsonWriter.java | 1 + poi/src/main/java/org/apache/poi/util/IOUtils.java | 36 +- poi/src/test/java/org/apache/poi/POITestCase.java | 65 +- .../poi/ss/usermodel/BaseTestBugzillaIssues.java | 8 +- .../org/apache/poi/ss/usermodel/BaseTestSheet.java | 11 +- .../poi/ss/usermodel/BaseTestSheetShiftRows.java | 997 ++++++++++----------- .../apache/poi/ss/usermodel/BaseTestWorkbook.java | 2 +- 19 files changed, 720 insertions(+), 790 deletions(-) (limited to 'poi/src') diff --git a/poi/src/main/java/org/apache/poi/hpsf/ClassID.java b/poi/src/main/java/org/apache/poi/hpsf/ClassID.java index aaf45554ae..21aba376b0 100644 --- a/poi/src/main/java/org/apache/poi/hpsf/ClassID.java +++ b/poi/src/main/java/org/apache/poi/hpsf/ClassID.java @@ -49,7 +49,7 @@ public class ClassID implements Duplicatable, GenericRecord { private final byte[] bytes = new byte[LENGTH]; /** - * Creates a {@link ClassID} and reads its value from a byte array. + * Creates a ClassID and reads its value from a byte array. * * @param src The byte array to read from. * @param offset The offset of the first byte to read. @@ -60,7 +60,7 @@ public class ClassID implements Duplicatable, GenericRecord { /** - * Creates a {@link ClassID} and initializes its value with 0x00 bytes. + * Creates a ClassID and initializes its value with 0x00 bytes. */ public ClassID() { Arrays.fill(bytes, (byte)0); @@ -77,7 +77,7 @@ public class ClassID implements Duplicatable, GenericRecord { /** - * Creates a {@link ClassID} from a human-readable representation of the Class ID in standard + * Creates a ClassID from a human-readable representation of the Class ID in standard * format {@code "{xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx}"}. * * @param externalForm representation of the Class ID represented by this object. @@ -102,12 +102,11 @@ public class ClassID implements Duplicatable, GenericRecord { /** * @return The number of bytes occupied by this object in the byte stream. */ + @SuppressWarnings("java:S1845") public int length() { return LENGTH; } - - /** * Gets the bytes making out the class ID. They are returned in correct order, i.e. big-endian. * @@ -117,8 +116,6 @@ public class ClassID implements Duplicatable, GenericRecord { return bytes; } - - /** * Sets the bytes making out the class ID. * @@ -129,8 +126,6 @@ public class ClassID implements Duplicatable, GenericRecord { System.arraycopy(bytes, 0, this.bytes, 0, LENGTH); } - - /** * Reads the class ID's value from a byte array by turning little-endian into big-endian. * @@ -249,10 +244,6 @@ public class ClassID implements Duplicatable, GenericRecord { ; } - - /** - * @see Object#hashCode() - */ @Override public int hashCode() { return toString().hashCode(); diff --git a/poi/src/main/java/org/apache/poi/hpsf/PropertySet.java b/poi/src/main/java/org/apache/poi/hpsf/PropertySet.java index 5237309c9c..eed2a7d4c8 100644 --- a/poi/src/main/java/org/apache/poi/hpsf/PropertySet.java +++ b/poi/src/main/java/org/apache/poi/hpsf/PropertySet.java @@ -514,51 +514,52 @@ public class PropertySet { } private byte[] toBytes() throws WritingNotSupportedException, IOException { - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - LittleEndianOutputStream leos = new LittleEndianOutputStream(bos); - - /* Write the number of sections in this property set stream. */ - final int nrSections = getSectionCount(); - - /* Write the property set's header. */ - leos.writeShort(getByteOrder()); - leos.writeShort(getFormat()); - leos.writeInt(getOSVersion()); - putClassId(bos, getClassID()); - leos.writeInt(nrSections); - - assert(bos.size() == OFFSET_HEADER); - - final int[][] offsets = new int[getSectionCount()][2]; - - /* Write the section list, i.e. the references to the sections. Each - * entry in the section list consist of the section's class ID and the - * section's offset relative to the beginning of the stream. */ - int secCnt = 0; - for (final Section section : getSections()) { - final ClassID formatID = section.getFormatID(); - if (formatID == null) { - throw new NoFormatIDException(); + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); + LittleEndianOutputStream leos = new LittleEndianOutputStream(bos)) { + + /* Write the number of sections in this property set stream. */ + final int nrSections = getSectionCount(); + + /* Write the property set's header. */ + leos.writeShort(getByteOrder()); + leos.writeShort(getFormat()); + leos.writeInt(getOSVersion()); + putClassId(bos, getClassID()); + leos.writeInt(nrSections); + + assert (bos.size() == OFFSET_HEADER); + + final int[][] offsets = new int[getSectionCount()][2]; + + /* Write the section list, i.e. the references to the sections. Each + * entry in the section list consist of the section's class ID and the + * section's offset relative to the beginning of the stream. */ + int secCnt = 0; + for (final Section section : getSections()) { + final ClassID formatID = section.getFormatID(); + if (formatID == null) { + throw new NoFormatIDException(); + } + putClassId(bos, formatID); + offsets[secCnt++][0] = bos.size(); + // offset dummy - filled later + leos.writeInt(-1); } - putClassId(bos, formatID); - offsets[secCnt++][0] = bos.size(); - // offset dummy - filled later - leos.writeInt(-1); - } - /* Write the sections themselves. */ - secCnt = 0; - for (final Section section : getSections()) { - offsets[secCnt++][1] = bos.size(); - section.write(bos); - } + /* Write the sections themselves. */ + secCnt = 0; + for (final Section section : getSections()) { + offsets[secCnt++][1] = bos.size(); + section.write(bos); + } - byte[] result = bos.toByteArray(); - for (int[] off : offsets) { - LittleEndian.putInt(result, off[0], off[1]); - } + byte[] result = bos.toByteArray(); + for (int[] off : offsets) { + LittleEndian.putInt(result, off[0], off[1]); + } - return result; + return result; + } } /** diff --git a/poi/src/main/java/org/apache/poi/hpsf/Section.java b/poi/src/main/java/org/apache/poi/hpsf/Section.java index 90d20cf448..13ae9af603 100644 --- a/poi/src/main/java/org/apache/poi/hpsf/Section.java +++ b/poi/src/main/java/org/apache/poi/hpsf/Section.java @@ -735,53 +735,54 @@ public class Section { } final int[][] offsets = new int[properties.size()][2]; - final UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - final LittleEndianOutputStream leos = new LittleEndianOutputStream(bos); + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); + LittleEndianOutputStream leos = new LittleEndianOutputStream(bos)) { - /* Write the section's length - dummy value, fixed later */ - leos.writeInt(-1); + /* Write the section's length - dummy value, fixed later */ + leos.writeInt(-1); - /* Write the section's number of properties: */ - leos.writeInt(properties.size()); + /* Write the section's number of properties: */ + leos.writeInt(properties.size()); - int propCnt = 0; - for (Property p : properties.values()) { - /* Write the property list entry. */ - leos.writeUInt(p.getID()); - // dummy offset to be fixed later - offsets[propCnt++][0] = bos.size(); - leos.writeInt(-1); - } + int propCnt = 0; + for (Property p : properties.values()) { + /* Write the property list entry. */ + leos.writeUInt(p.getID()); + // dummy offset to be fixed later + offsets[propCnt++][0] = bos.size(); + leos.writeInt(-1); + } - /* Write the properties and the property list into their respective - * streams: */ - propCnt = 0; - for (Property p : properties.values()) { - offsets[propCnt++][1] = bos.size(); - /* If the property ID is not equal 0 we write the property and all - * is fine. However, if it equals 0 we have to write the section's - * dictionary which has an implicit type only and an explicit - * value. */ - if (p.getID() != 0) { - /* Write the property and update the position to the next - * property. */ - p.write(bos, codepage); - } else { - writeDictionary(bos, codepage); + /* Write the properties and the property list into their respective + * streams: */ + propCnt = 0; + for (Property p : properties.values()) { + offsets[propCnt++][1] = bos.size(); + /* If the property ID is not equal 0 we write the property and all + * is fine. However, if it equals 0 we have to write the section's + * dictionary which has an implicit type only and an explicit + * value. */ + if (p.getID() != 0) { + /* Write the property and update the position to the next + * property. */ + p.write(bos, codepage); + } else { + writeDictionary(bos, codepage); + } } - } - byte[] result = bos.toByteArray(); - LittleEndian.putInt(result, 0, bos.size()); + byte[] result = bos.toByteArray(); + LittleEndian.putInt(result, 0, bos.size()); - for (int[] off : offsets) { - LittleEndian.putUInt(result, off[0], off[1]); - } + for (int[] off : offsets) { + LittleEndian.putUInt(result, off[0], off[1]); + } - out.write(result); + out.write(result); - return bos.size(); + return bos.size(); + } } /** diff --git a/poi/src/main/java/org/apache/poi/hssf/record/RecordInputStream.java b/poi/src/main/java/org/apache/poi/hssf/record/RecordInputStream.java index 94fc4070c7..5b7b114cdb 100644 --- a/poi/src/main/java/org/apache/poi/hssf/record/RecordInputStream.java +++ b/poi/src/main/java/org/apache/poi/hssf/record/RecordInputStream.java @@ -459,17 +459,20 @@ public final class RecordInputStream implements LittleEndianInput { */ @Deprecated public byte[] readAllContinuedRemainder() { - UnsynchronizedByteArrayOutputStream out = new UnsynchronizedByteArrayOutputStream(2 * MAX_RECORD_DATA_SIZE); + try (UnsynchronizedByteArrayOutputStream out = new UnsynchronizedByteArrayOutputStream(2 * MAX_RECORD_DATA_SIZE)) { - while (true) { - byte[] b = readRemainder(); - out.write(b, 0, b.length); - if (!isContinueNext()) { - break; + while (true) { + byte[] b = readRemainder(); + out.write(b, 0, b.length); + if (!isContinueNext()) { + break; + } + nextRecord(); } - nextRecord(); + return out.toByteArray(); + } catch (IOException ex) { + throw new RecordFormatException(ex); } - return out.toByteArray(); } /** The remaining number of bytes in the current record. diff --git a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingRule.java b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingRule.java index 29506743f6..af688471fd 100644 --- a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingRule.java +++ b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingRule.java @@ -64,8 +64,8 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin * Only newer style formatting rules have priorities. For older ones, * we don't know priority for these, other than definition/model order, * which appears to be what Excel uses. - * @see org.apache.poi.ss.usermodel.ConditionalFormattingRule#getPriority() */ + @Override public int getPriority() { CFRule12Record rule12 = getCFRule12Record(false); if (rule12 == null) return 0; @@ -74,8 +74,8 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin /** * Always true for HSSF files, per Microsoft Excel documentation - * @see org.apache.poi.ss.usermodel.ConditionalFormattingRule#getStopIfTrue() */ + @Override public boolean getStopIfTrue() { return true; } @@ -95,8 +95,8 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin /** * Always null for HSSF records, until someone figures out where to find it - * @see org.apache.poi.ss.usermodel.ConditionalFormattingRule#getNumberFormat() */ + @Override public ExcelNumberFormat getNumberFormat() { return null; } @@ -112,16 +112,18 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin } /** - * @return - font formatting object if defined, null otherwise + * @return - font formatting object if defined, {@code null} otherwise */ + @Override public HSSFFontFormatting getFontFormatting() { return getFontFormatting(false); } /** * create a new font formatting structure if it does not exist, * otherwise just return existing object. - * @return - font formatting object, never returns null. + * @return - font formatting object, never returns {@code null}. */ + @Override public HSSFFontFormatting createFontFormatting() { return getFontFormatting(true); } @@ -137,16 +139,18 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin } /** - * @return - border formatting object if defined, null otherwise + * @return - border formatting object if defined, {@code null} otherwise */ + @Override public HSSFBorderFormatting getBorderFormatting() { return getBorderFormatting(false); } /** * create a new border formatting structure if it does not exist, * otherwise just return existing object. - * @return - border formatting object, never returns null. + * @return - border formatting object, never returns {@code null}. */ + @Override public HSSFBorderFormatting createBorderFormatting() { return getBorderFormatting(true); } @@ -162,8 +166,9 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin } /** - * @return - pattern formatting object if defined, null otherwise + * @return - pattern formatting object if defined, {@code null} otherwise */ + @Override public HSSFPatternFormatting getPatternFormatting() { return getPatternFormatting(false); @@ -171,8 +176,9 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin /** * create a new pattern formatting structure if it does not exist, * otherwise just return existing object. - * @return - pattern formatting object, never returns null. + * @return - pattern formatting object, never returns {@code null}. */ + @Override public HSSFPatternFormatting createPatternFormatting() { return getPatternFormatting(true); @@ -192,8 +198,9 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin } /** - * @return databar / data-bar formatting object if defined, null otherwise + * @return databar / data-bar formatting object if defined, {@code null} otherwise */ + @Override public HSSFDataBarFormatting getDataBarFormatting() { return getDataBarFormatting(false); } @@ -218,8 +225,9 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin } /** - * @return icon / multi-state formatting object if defined, null otherwise + * @return icon / multi-state formatting object if defined, {@code null} otherwise */ + @Override public HSSFIconMultiStateFormatting getMultiStateFormatting() { return getMultiStateFormatting(false); } @@ -245,8 +253,9 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin } /** - * @return color scale / gradient formatting object if defined, null otherwise + * @return color scale / gradient formatting object if defined, {@code null} otherwise */ + @Override public HSSFColorScaleFormatting getColorScaleFormatting() { return getColorScaleFormatting(false); } @@ -270,12 +279,13 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin /** * always null (not a filter condition) or {@link ConditionFilterType#FILTER} if it is. - * @see org.apache.poi.ss.usermodel.ConditionalFormattingRule#getConditionFilterType() */ + @Override public ConditionFilterType getConditionFilterType() { return getConditionType() == ConditionType.FILTER ? ConditionFilterType.FILTER : null; } + @Override public ConditionFilterData getFilterConfiguration() { return null; } @@ -288,11 +298,13 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin return cfRuleRecord.getComparisonOperation(); } + @Override public String getFormula1() { return toFormulaString(cfRuleRecord.getParsedExpression1()); } + @Override public String getFormula2() { byte conditionType = cfRuleRecord.getConditionType(); if (conditionType == CELL_COMPARISON) { @@ -306,6 +318,7 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin return null; } + @Override public String getText() { return null; // not available here, unless it exists and is unimplemented in cfRuleRecord } @@ -325,6 +338,7 @@ public final class HSSFConditionalFormattingRule implements ConditionalFormattin * Conditional format rules don't define stripes, so always 0 * @see org.apache.poi.ss.usermodel.DifferentialStyleProvider#getStripeSize() */ + @Override public int getStripeSize() { return 0; } diff --git a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingThreshold.java b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingThreshold.java index 4ab124c05c..987d9f47f3 100644 --- a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingThreshold.java +++ b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFConditionalFormattingThreshold.java @@ -31,32 +31,40 @@ public final class HSSFConditionalFormattingThreshold implements org.apache.poi. private final HSSFSheet sheet; private final HSSFWorkbook workbook; - protected HSSFConditionalFormattingThreshold(Threshold threshold, HSSFSheet sheet) { + HSSFConditionalFormattingThreshold(Threshold threshold, HSSFSheet sheet) { this.threshold = threshold; this.sheet = sheet; this.workbook = sheet.getWorkbook(); } - protected Threshold getThreshold() { + + Threshold getThreshold() { return threshold; } + @Override public RangeType getRangeType() { return RangeType.byId(threshold.getType()); } + @Override public void setRangeType(RangeType type) { threshold.setType((byte)type.id); } + @Override public String getFormula() { return toFormulaString(threshold.getParsedExpression(), workbook); } + @Override public void setFormula(String formula) { threshold.setParsedExpression(parseFormula(formula, sheet)); } + @Override public Double getValue() { return threshold.getValue(); } + + @Override public void setValue(Double value) { threshold.setValue(value); } diff --git a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFIconMultiStateFormatting.java b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFIconMultiStateFormatting.java index cabfad2f80..110612e8bb 100644 --- a/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFIconMultiStateFormatting.java +++ b/poi/src/main/java/org/apache/poi/hssf/usermodel/HSSFIconMultiStateFormatting.java @@ -24,41 +24,46 @@ import org.apache.poi.hssf.record.cf.Threshold; import org.apache.poi.ss.usermodel.ConditionalFormattingThreshold; /** - * High level representation for Icon / Multi-State Formatting + * High level representation for Icon / Multi-State Formatting * component of Conditional Formatting settings */ public final class HSSFIconMultiStateFormatting implements org.apache.poi.ss.usermodel.IconMultiStateFormatting { private final HSSFSheet sheet; - private final CFRule12Record cfRule12Record; private final IconMultiStateFormatting iconFormatting; - protected HSSFIconMultiStateFormatting(CFRule12Record cfRule12Record, HSSFSheet sheet) { + HSSFIconMultiStateFormatting(CFRule12Record cfRule12Record, HSSFSheet sheet) { this.sheet = sheet; - this.cfRule12Record = cfRule12Record; - this.iconFormatting = this.cfRule12Record.getMultiStateFormatting(); + this.iconFormatting = cfRule12Record.getMultiStateFormatting(); } + @Override public IconSet getIconSet() { return iconFormatting.getIconSet(); } + @Override public void setIconSet(IconSet set) { iconFormatting.setIconSet(set); } + @Override public boolean isIconOnly() { return iconFormatting.isIconOnly(); } + @Override public void setIconOnly(boolean only) { iconFormatting.setIconOnly(only); } + @Override public boolean isReversed() { return iconFormatting.isReversed(); } + @Override public void setReversed(boolean reversed) { iconFormatting.setReversed(reversed); } + @Override public HSSFConditionalFormattingThreshold[] getThresholds() { Threshold[] t = iconFormatting.getThresholds(); HSSFConditionalFormattingThreshold[] ht = new HSSFConditionalFormattingThreshold[t.length]; @@ -68,6 +73,7 @@ public final class HSSFIconMultiStateFormatting implements org.apache.poi.ss.use return ht; } + @Override public void setThresholds(ConditionalFormattingThreshold[] thresholds) { Threshold[] t = new Threshold[thresholds.length]; for (int i=0; i= MAX_STRING_LENGTH) { - LOGGER.atWarn().log("stopped reading unicode name after {} bytes", box(read)); + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream()) { + int b0 = IOUtils.readByte(is); + int b1 = IOUtils.readByte(is); + + int read = 2; + while ((b0 + b1) != 0 && read < MAX_STRING_LENGTH) { + bos.write(b0); + bos.write(b1); + b0 = IOUtils.readByte(is); + b1 = IOUtils.readByte(is); + read += 2; + } + if (read >= MAX_STRING_LENGTH) { + LOGGER.atWarn().log("stopped reading unicode name after {} bytes", box(read)); + } + return bos.toString(StandardCharsets.UTF_16LE); } - return bos.toString(StandardCharsets.UTF_16LE); } private static String readMBCS(int firstByte, InputStream is, Charset charset) throws IOException { - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - int len = 0; - int b = firstByte; - while (b > 0 && len < MAX_STRING_LENGTH) { - ++len; - bos.write(b); - b = IOUtils.readByte(is); - } - return bos.toString(charset); + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream()) { + int len = 0; + int b = firstByte; + while (b > 0 && len < MAX_STRING_LENGTH) { + ++len; + bos.write(b); + b = IOUtils.readByte(is); + } + return bos.toString(charset); + } } /** @@ -792,9 +794,7 @@ public class VBAMacroReader implements Closeable { */ private static byte[] findCompressedStreamWBruteForce(InputStream is) throws IOException { //buffer to memory for multiple tries - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - IOUtils.copy(is, bos); - byte[] compressed = bos.toByteArray(); + byte[] compressed = IOUtils.toByteArray(is); byte[] decompressed = null; for (int i = 0; i < compressed.length; i++) { if (compressed[i] == 0x01 && i < compressed.length-1) { @@ -821,12 +821,10 @@ public class VBAMacroReader implements Closeable { } private static byte[] tryToDecompress(InputStream is) { - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(); - try { - IOUtils.copy(new RLEDecompressingInputStream(is), bos); + try (RLEDecompressingInputStream ris = new RLEDecompressingInputStream(is)) { + return IOUtils.toByteArray(ris); } catch (IllegalArgumentException | IOException | IllegalStateException e){ return null; } - return bos.toByteArray(); } } diff --git a/poi/src/main/java/org/apache/poi/ss/extractor/EmbeddedExtractor.java b/poi/src/main/java/org/apache/poi/ss/extractor/EmbeddedExtractor.java index 18eef73108..47793a14c1 100644 --- a/poi/src/main/java/org/apache/poi/ss/extractor/EmbeddedExtractor.java +++ b/poi/src/main/java/org/apache/poi/ss/extractor/EmbeddedExtractor.java @@ -162,14 +162,13 @@ public class EmbeddedExtractor implements Iterable { protected EmbeddedData extract(DirectoryNode dn) throws IOException { assert(canExtract(dn)); - UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(20000); - try (POIFSFileSystem dest = new POIFSFileSystem()) { + try (UnsynchronizedByteArrayOutputStream bos = new UnsynchronizedByteArrayOutputStream(20000); + POIFSFileSystem dest = new POIFSFileSystem()) { copyNodes(dn, dest.getRoot()); // start with a reasonable big size dest.writeFilesystem(bos); + return new EmbeddedData(dn.getName(), bos.toByteArray(), CONTENT_TYPE_BYTES); } - - return new EmbeddedData(dn.getName(), bos.toByteArray(), CONTENT_TYPE_BYTES); } protected EmbeddedData extract(Picture source) throws IOException { diff --git a/poi/src/main/java/org/apache/poi/ss/usermodel/ConditionalFormattingThreshold.java b/poi/src/main/java/org/apache/poi/ss/usermodel/ConditionalFormattingThreshold.java index c70e3d1d24..69cc1887c5 100644 --- a/poi/src/main/java/org/apache/poi/ss/usermodel/ConditionalFormattingThreshold.java +++ b/poi/src/main/java/org/apache/poi/ss/usermodel/ConditionalFormattingThreshold.java @@ -27,7 +27,7 @@ package org.apache.poi.ss.usermodel; * icon and which Yellow or Red.

*/ public interface ConditionalFormattingThreshold { - public enum RangeType { + enum RangeType { /** Number / Parameter */ NUMBER(1, "num"), /** The minimum value from the range */ @@ -41,16 +41,16 @@ public interface ConditionalFormattingThreshold { UNALLOCATED(6, null), /** Formula result */ FORMULA(7, "formula"); - + /** Numeric ID of the type */ public final int id; /** Name (system) of the type */ public final String name; - + public String toString() { return id + " - " + name; } - + public static RangeType byId(int id) { return values()[id-1]; // 1-based IDs } @@ -60,51 +60,51 @@ public interface ConditionalFormattingThreshold { } return null; } - - private RangeType(int id, String name) { + + RangeType(int id, String name) { this.id = id; this.name = name; } } - + /** * Get the Range Type used */ RangeType getRangeType(); - + /** * Changes the Range Type used - * + * *

If you change the range type, you need to * ensure that the Formula and Value parameters * are compatible with it before saving

*/ void setRangeType(RangeType type); - + /** * Formula to use to calculate the threshold, - * or null if no formula + * or {@code null} if no formula */ String getFormula(); /** * Sets the formula used to calculate the threshold, - * or unsets it if null is given. + * or unsets it if {@code null} is given. */ void setFormula(String formula); - + /** - * Gets the value used for the threshold, or - * null if there isn't one. + * Gets the value used for the threshold, or + * {@code null} if there isn't one. */ Double getValue(); - + /** - * Sets the value used for the threshold. - *

If the type is {@link RangeType#PERCENT} or + * Sets the value used for the threshold. + *

If the type is {@link RangeType#PERCENT} or * {@link RangeType#PERCENTILE} it must be between 0 and 100. *

If the type is {@link RangeType#MIN} or {@link RangeType#MAX} * or {@link RangeType#FORMULA} it shouldn't be set. - *

Use null to unset + *

Use {@code null} to unset */ void setValue(Double value); } diff --git a/poi/src/main/java/org/apache/poi/util/GenericRecordJsonWriter.java b/poi/src/main/java/org/apache/poi/util/GenericRecordJsonWriter.java index 281b8180b0..f282907c94 100644 --- a/poi/src/main/java/org/apache/poi/util/GenericRecordJsonWriter.java +++ b/poi/src/main/java/org/apache/poi/util/GenericRecordJsonWriter.java @@ -265,6 +265,7 @@ public class GenericRecordJsonWriter implements Closeable { return true; } + @SuppressWarnings("java:S3516") protected boolean printNumber(String name, Object o) { Number n = (Number)o; printName(name); diff --git a/poi/src/main/java/org/apache/poi/util/IOUtils.java b/poi/src/main/java/org/apache/poi/util/IOUtils.java index 279b56974c..09bb25a9cb 100644 --- a/poi/src/main/java/org/apache/poi/util/IOUtils.java +++ b/poi/src/main/java/org/apache/poi/util/IOUtils.java @@ -178,29 +178,29 @@ public final class IOUtils { } final int len = Math.min(length, maxLength); - UnsynchronizedByteArrayOutputStream baos = new UnsynchronizedByteArrayOutputStream(len == Integer.MAX_VALUE ? 4096 : len); + try (UnsynchronizedByteArrayOutputStream baos = new UnsynchronizedByteArrayOutputStream(len == Integer.MAX_VALUE ? 4096 : len)) { + byte[] buffer = new byte[4096]; + int totalBytes = 0, readBytes; + do { + readBytes = stream.read(buffer, 0, Math.min(buffer.length, len - totalBytes)); + totalBytes += Math.max(readBytes, 0); + if (readBytes > 0) { + baos.write(buffer, 0, readBytes); + } - byte[] buffer = new byte[4096]; - int totalBytes = 0, readBytes; - do { - readBytes = stream.read(buffer, 0, Math.min(buffer.length, len-totalBytes)); - totalBytes += Math.max(readBytes,0); - if (readBytes > 0) { - baos.write(buffer, 0, readBytes); - } + checkByteSizeLimit(totalBytes); + } while (totalBytes < len && readBytes > -1); - checkByteSizeLimit(totalBytes); - } while (totalBytes < len && readBytes > -1); + if (maxLength != Integer.MAX_VALUE && totalBytes == maxLength) { + throw new IOException("MaxLength (" + maxLength + ") reached - stream seems to be invalid."); + } - if (maxLength != Integer.MAX_VALUE && totalBytes == maxLength) { - throw new IOException("MaxLength ("+maxLength+") reached - stream seems to be invalid."); - } + if (len != Integer.MAX_VALUE && totalBytes < len) { + throw new EOFException("unexpected EOF - expected len: " + len + " - actual len: " + totalBytes); + } - if (len != Integer.MAX_VALUE && totalBytes < len) { - throw new EOFException("unexpected EOF - expected len: "+len+" - actual len: "+totalBytes); + return baos.toByteArray(); } - - return baos.toByteArray(); } private static void checkLength(long length, int maxLength) { diff --git a/poi/src/test/java/org/apache/poi/POITestCase.java b/poi/src/test/java/org/apache/poi/POITestCase.java index 7dbed7bd0d..4f43cb216c 100644 --- a/poi/src/test/java/org/apache/poi/POITestCase.java +++ b/poi/src/test/java/org/apache/poi/POITestCase.java @@ -25,8 +25,6 @@ import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertTrue; -import static org.junit.jupiter.api.Assertions.fail; -import static org.junit.jupiter.api.Assumptions.assumeTrue; import java.io.File; import java.lang.reflect.Field; @@ -47,6 +45,7 @@ import org.mockito.internal.matchers.apachecommons.ReflectionEquals; * Util class for POI JUnit TestCases, which provide additional features */ @Internal +@SuppressWarnings("java:S2187") public final class POITestCase { private POITestCase() { @@ -141,68 +140,6 @@ public final class POITestCase { assertTrue(new ReflectionEquals(expected, "$jacocoData").matches(actual)); } - /** - * Rather than adding {@literal @}Ignore to known-failing tests, - * write the test so that it notifies us if it starts passing. - * This is useful for closing related or forgotten bugs. - * - * An Example: - * 

-     * public static int add(int a, int b) {
-     *     // a known bug in behavior that has not been fixed yet
-     *     raise UnsupportedOperationException("add");
-     * }
-     *
-     * {@literal @}Test
-     * void knownFailingUnitTest() {
-     *     try {
-     *         assertEquals(2, add(1,1));
-     *         // this test fails because the assumption that this bug had not been fixed is false
-     *         testPassesNow(12345);
-     *     } catch (UnsupportedOperationException e) {
-     *         // test is skipped because the assumption that this bug had not been fixed is true
-     *         skipTest(e);
-     *     }
-     * }
-     *
-     * Once passing, this unit test can be rewritten as:
-     * {@literal @}Test
-     * void knownPassingUnitTest() {
-     *     assertEquals(2, add(1,1));
-     * }
-     *
-     * If you have a better idea how to simplify test code while still notifying
-     * us when a previous known-failing test now passes, please improve these.
-     * As a bonus, a known-failing test that fails should not be counted as a
-     * passing test.
-     *
-     * One possible alternative is to expect the known exception, but without
-     * a clear message that it is a good thing to no longer get the expected
-     * exception once the test passes.
-     * {@literal @}Test(expected=UnsupportedOperationException.class)
-     * void knownFailingUnitTest() {
-     *     assertEquals(2, add(1,1));
-     * }
-     *
-     * @param e  the exception that was caught that will no longer
-     * be raised when the bug is fixed
-     */
-    public static void skipTest(Throwable e) {
-        assumeTrue(e != null, "This test currently fails with");
-    }
-    /**
-     * @see #skipTest(Throwable)
-     *
-     * @param bug  the bug number corresponding to a known bug in bugzilla
-     */
-    public static void testPassesNow(int bug) {
-        fail("This test passes now. Please update the unit test and bug " + bug + ".");
-    }
-
-    public static void assertBetween(String message, int value, int min, int max) {
-        assertTrue(min <= value, message + ": " + value + " is less than the minimum value of " + min);
-        assertTrue(value <= max, message + ": " + value + " is greater than the maximum value of " + max);
-    }
 
     /**
      * Ensures that the temporary directory is defined and exists and
diff --git a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java
index 5e7b5d87f7..3edce0e0e4 100644
--- a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java
+++ b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestBugzillaIssues.java
@@ -1382,6 +1382,7 @@ public abstract class BaseTestBugzillaIssues {
     }
 
     @Test
+    @SuppressWarnings("java:S2699")
     void test58896() throws IOException {
         final int nrows = 160;
         final int ncols = 139;
@@ -1493,8 +1494,8 @@ public abstract class BaseTestBugzillaIssues {
 
             // *******************************
             // First cell of array formula, OK
-            int rowId = 0;
-            int cellId = 1;
+            final int rowId = 0;
+            final int cellId = 1;
 
             Row row = sheet.getRow(rowId);
             Cell cell = row.getCell(cellId);
@@ -1507,9 +1508,6 @@ public abstract class BaseTestBugzillaIssues {
 
             // *******************************
             // Second cell of array formula, NOT OK for xlsx files
-            rowId = 1;
-            cellId = 1;
-
             row = sheet.getRow(rowId);
             cell = row.getCell(cellId);
             assertEquals("A1", cell.getCellFormula());
diff --git a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestSheet.java b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestSheet.java
index 947bd71a9f..8ed16c996b 100644
--- a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestSheet.java
+++ b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestSheet.java
@@ -17,7 +17,6 @@
 
 package org.apache.poi.ss.usermodel;
 
-import static org.apache.poi.POITestCase.assertBetween;
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
@@ -50,8 +49,7 @@ import org.junit.jupiter.api.Test;
 
 /**
  * Common superclass for testing {@link org.apache.poi.hssf.usermodel.HSSFCell},
- * {@link org.apache.poi.xssf.usermodel.XSSFCell} and
- * {@link org.apache.poi.xssf.streaming.SXSSFCell}
+ * XSSFCell and SXSSFCell
  */
 public abstract class BaseTestSheet {
     private static final int ROW_COUNT = 40000;
@@ -430,7 +428,7 @@ public abstract class BaseTestSheet {
             assertCollectionEquals(mergedRegions.values(), sheet.getMergedRegions());
 
             Collection removed = Arrays.asList(0, 2, 3, 6, 8);
-            mergedRegions.keySet().removeAll(removed);
+            removed.forEach(mergedRegions.keySet()::remove);
             sheet.removeMergedRegions(removed);
             assertCollectionEquals(mergedRegions.values(), sheet.getMergedRegions());
         }
@@ -1364,4 +1362,9 @@ public abstract class BaseTestSheet {
             assertBetween("Date column width", s.getColumnWidth(1), 4750, 7300);
         }
     }
+
+    private static void assertBetween(String message, int value, int min, int max) {
+        assertTrue(min <= value, message + ": " + value + " is less than the minimum value of " + min);
+        assertTrue(value <= max, message + ": " + value + " is greater than the maximum value of " + max);
+    }
 }
diff --git a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java
index 6c8e2f2a93..869b591eff 100644
--- a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java
+++ b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestSheetShiftRows.java
@@ -17,8 +17,6 @@
 
 package org.apache.poi.ss.usermodel;
 
-import static org.apache.poi.POITestCase.skipTest;
-import static org.apache.poi.POITestCase.testPassesNow;
 import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
@@ -60,57 +58,55 @@ public abstract class BaseTestSheetShiftRows {
     public final void testShiftRows() throws IOException {
         // Read initial file in
         String sampleName = "SimpleMultiCell." + _testDataProvider.getStandardFileNameExtension();
-        Workbook wb1 = _testDataProvider.openSampleWorkbook(sampleName);
-        Sheet s = wb1.getSheetAt( 0 );
-
-        // Shift the second row down 1 and write to temp file
-        s.shiftRows( 1, 1, 1 );
-
-        Workbook wb2 = _testDataProvider.writeOutAndReadBack(wb1);
-        wb1.close();
-
-        // Read from temp file and check the number of cells in each
-        // row (in original file each row was unique)
-        s = wb2.getSheetAt( 0 );
-
-        assertEquals(s.getRow(0).getPhysicalNumberOfCells(), 1);
-        confirmEmptyRow(s, 1);
-        assertEquals(s.getRow(2).getPhysicalNumberOfCells(), 2);
-        assertEquals(s.getRow(3).getPhysicalNumberOfCells(), 4);
-        assertEquals(s.getRow(4).getPhysicalNumberOfCells(), 5);
-
-        // Shift rows 1-3 down 3 in the current one.  This tests when
-        // 1 row is blank.  Write to a another temp file
-        s.shiftRows( 0, 2, 3 );
-        Workbook wb3 = _testDataProvider.writeOutAndReadBack(wb2);
-        wb2.close();
-
-        // Read and ensure things are where they should be
-        s = wb3.getSheetAt(0);
-        confirmEmptyRow(s, 0);
-        confirmEmptyRow(s, 1);
-        confirmEmptyRow(s, 2);
-        assertEquals(s.getRow(3).getPhysicalNumberOfCells(), 1);
-        confirmEmptyRow(s, 4);
-        assertEquals(s.getRow(5).getPhysicalNumberOfCells(), 2);
-
-        wb3.close();
+        try (Workbook wb1 = _testDataProvider.openSampleWorkbook(sampleName)) {
+            Sheet s = wb1.getSheetAt(0);
+
+            // Shift the second row down 1 and write to temp file
+            s.shiftRows(1, 1, 1);
+
+            try (Workbook wb2 = _testDataProvider.writeOutAndReadBack(wb1)) {
+
+                // Read from temp file and check the number of cells in each
+                // row (in original file each row was unique)
+                s = wb2.getSheetAt(0);
+
+                assertEquals(s.getRow(0).getPhysicalNumberOfCells(), 1);
+                confirmEmptyRow(s, 1);
+                assertEquals(s.getRow(2).getPhysicalNumberOfCells(), 2);
+                assertEquals(s.getRow(3).getPhysicalNumberOfCells(), 4);
+                assertEquals(s.getRow(4).getPhysicalNumberOfCells(), 5);
+
+                // Shift rows 1-3 down 3 in the current one.  This tests when
+                // 1 row is blank.  Write to a another temp file
+                s.shiftRows(0, 2, 3);
+                try (Workbook wb3 = _testDataProvider.writeOutAndReadBack(wb2)) {
+                    // Read and ensure things are where they should be
+                    s = wb3.getSheetAt(0);
+                    confirmEmptyRow(s, 0);
+                    confirmEmptyRow(s, 1);
+                    confirmEmptyRow(s, 2);
+                    assertEquals(s.getRow(3).getPhysicalNumberOfCells(), 1);
+                    confirmEmptyRow(s, 4);
+                    assertEquals(s.getRow(5).getPhysicalNumberOfCells(), 2);
+                }
+            }
+        }
 
         // Read the first file again
-        Workbook wb4 = _testDataProvider.openSampleWorkbook(sampleName);
-        s = wb4.getSheetAt( 0 );
-
-        // Shift rows 3 and 4 up and write to temp file
-        s.shiftRows( 2, 3, -2 );
-        Workbook wb5 = _testDataProvider.writeOutAndReadBack(wb4);
-        wb4.close();
-        s = wb5.getSheetAt( 0 );
-        assertEquals(s.getRow(0).getPhysicalNumberOfCells(), 3);
-        assertEquals(s.getRow(1).getPhysicalNumberOfCells(), 4);
-        confirmEmptyRow(s, 2);
-        confirmEmptyRow(s, 3);
-        assertEquals(s.getRow(4).getPhysicalNumberOfCells(), 5);
-        wb5.close();
+        try (Workbook wb4 = _testDataProvider.openSampleWorkbook(sampleName)) {
+            Sheet s = wb4.getSheetAt(0);
+
+            // Shift rows 3 and 4 up and write to temp file
+            s.shiftRows(2, 3, -2);
+            try (Workbook wb5 = _testDataProvider.writeOutAndReadBack(wb4)) {
+                s = wb5.getSheetAt(0);
+                assertEquals(s.getRow(0).getPhysicalNumberOfCells(), 3);
+                assertEquals(s.getRow(1).getPhysicalNumberOfCells(), 4);
+                confirmEmptyRow(s, 2);
+                confirmEmptyRow(s, 3);
+                assertEquals(s.getRow(4).getPhysicalNumberOfCells(), 5);
+            }
+        }
     }
     private static void confirmEmptyRow(Sheet s, int rowIx) {
         Row row = s.getRow(rowIx);
@@ -149,195 +145,188 @@ public abstract class BaseTestSheetShiftRows {
 
     @Test
     void testShiftWithComments() throws IOException {
-        Workbook wb1 = _testDataProvider.openSampleWorkbook("comments." + _testDataProvider.getStandardFileNameExtension());
-
-        Sheet sheet = wb1.getSheet("Sheet1");
-        assertEquals(3, sheet.getLastRowNum());
-
-        // Verify comments are in the position expected
-        assertNotNull(sheet.getCellComment(new CellAddress(0,0)));
-        assertNull(sheet.getCellComment(new CellAddress(1,0)));
-        assertNotNull(sheet.getCellComment(new CellAddress(2,0)));
-        assertNotNull(sheet.getCellComment(new CellAddress(3,0)));
-
-        String comment1 = sheet.getCellComment(new CellAddress(0,0)).getString().getString();
-        assertEquals(comment1,"comment top row1 (index0)\n");
-        String comment3 = sheet.getCellComment(new CellAddress(2,0)).getString().getString();
-        assertEquals(comment3,"comment top row3 (index2)\n");
-        String comment4 = sheet.getCellComment(new CellAddress(3,0)).getString().getString();
-        assertEquals(comment4,"comment top row4 (index3)\n");
-
-        //Workbook wbBack = _testDataProvider.writeOutAndReadBack(wb);
-
-        // Shifting all but first line down to test comments shifting
-        sheet.shiftRows(1, sheet.getLastRowNum(), 1, true, true);
-
-        // Test that comments were shifted as expected
-        assertEquals(4, sheet.getLastRowNum());
-        assertNotNull(sheet.getCellComment(new CellAddress(0,0)));
-        assertNull(sheet.getCellComment(new CellAddress(1,0)));
-        assertNull(sheet.getCellComment(new CellAddress(2,0)));
-        assertNotNull(sheet.getCellComment(new CellAddress(3,0)));
-        assertNotNull(sheet.getCellComment(new CellAddress(4,0)));
-
-        String comment1_shifted = sheet.getCellComment(new CellAddress(0,0)).getString().getString();
-        assertEquals(comment1,comment1_shifted);
-        String comment3_shifted = sheet.getCellComment(new CellAddress(3,0)).getString().getString();
-        assertEquals(comment3,comment3_shifted);
-        String comment4_shifted = sheet.getCellComment(new CellAddress(4,0)).getString().getString();
-        assertEquals(comment4,comment4_shifted);
-
-        // Write out and read back in again
-        // Ensure that the changes were persisted
-        Workbook wb2 = _testDataProvider.writeOutAndReadBack(wb1);
-        wb1.close();
-
-        sheet = wb2.getSheet("Sheet1");
-        assertEquals(4, sheet.getLastRowNum());
-
-        // Verify comments are in the position expected after the shift
-        assertNotNull(sheet.getCellComment(new CellAddress(0,0)));
-        assertNull(sheet.getCellComment(new CellAddress(1,0)));
-        assertNull(sheet.getCellComment(new CellAddress(2,0)));
-        assertNotNull(sheet.getCellComment(new CellAddress(3,0)));
-        assertNotNull(sheet.getCellComment(new CellAddress(4,0)));
-
-        comment1_shifted = sheet.getCellComment(new CellAddress(0,0)).getString().getString();
-        assertEquals(comment1,comment1_shifted);
-        comment3_shifted = sheet.getCellComment(new CellAddress(3,0)).getString().getString();
-        assertEquals(comment3,comment3_shifted);
-        comment4_shifted = sheet.getCellComment(new CellAddress(4,0)).getString().getString();
-        assertEquals(comment4,comment4_shifted);
-
-        // Shifting back up again, now two rows
-        sheet.shiftRows(2, sheet.getLastRowNum(), -2, true, true);
-
-        // TODO: it seems HSSFSheet does not correctly remove comments from rows that are overwritten
-        // by shifting rows...
-        if(!(wb2 instanceof HSSFWorkbook)) {
-            assertEquals(2, sheet.getLastRowNum());
+        try (Workbook wb1 = _testDataProvider.openSampleWorkbook("comments." + _testDataProvider.getStandardFileNameExtension())) {
+
+            Sheet sheet = wb1.getSheet("Sheet1");
+            assertEquals(3, sheet.getLastRowNum());
 
             // Verify comments are in the position expected
-            assertNull(sheet.getCellComment(new CellAddress(0,0)),
-                "Had: " + (sheet.getCellComment(new CellAddress(0,0)) == null ? "null" : sheet.getCellComment(new CellAddress(0,0)).getString()));
-            assertNotNull(sheet.getCellComment(new CellAddress(1,0)));
-            assertNotNull(sheet.getCellComment(new CellAddress(2,0)));
+            assertNotNull(sheet.getCellComment(new CellAddress(0, 0)));
+            assertNull(sheet.getCellComment(new CellAddress(1, 0)));
+            assertNotNull(sheet.getCellComment(new CellAddress(2, 0)));
+            assertNotNull(sheet.getCellComment(new CellAddress(3, 0)));
+
+            String comment1 = sheet.getCellComment(new CellAddress(0, 0)).getString().getString();
+            assertEquals(comment1, "comment top row1 (index0)\n");
+            String comment3 = sheet.getCellComment(new CellAddress(2, 0)).getString().getString();
+            assertEquals(comment3, "comment top row3 (index2)\n");
+            String comment4 = sheet.getCellComment(new CellAddress(3, 0)).getString().getString();
+            assertEquals(comment4, "comment top row4 (index3)\n");
+
+            //Workbook wbBack = _testDataProvider.writeOutAndReadBack(wb);
+
+            // Shifting all but first line down to test comments shifting
+            sheet.shiftRows(1, sheet.getLastRowNum(), 1, true, true);
+
+            // Test that comments were shifted as expected
+            assertEquals(4, sheet.getLastRowNum());
+            assertNotNull(sheet.getCellComment(new CellAddress(0, 0)));
+            assertNull(sheet.getCellComment(new CellAddress(1, 0)));
+            assertNull(sheet.getCellComment(new CellAddress(2, 0)));
+            assertNotNull(sheet.getCellComment(new CellAddress(3, 0)));
+            assertNotNull(sheet.getCellComment(new CellAddress(4, 0)));
+
+            String comment1_shifted = sheet.getCellComment(new CellAddress(0, 0)).getString().getString();
+            assertEquals(comment1, comment1_shifted);
+            String comment3_shifted = sheet.getCellComment(new CellAddress(3, 0)).getString().getString();
+            assertEquals(comment3, comment3_shifted);
+            String comment4_shifted = sheet.getCellComment(new CellAddress(4, 0)).getString().getString();
+            assertEquals(comment4, comment4_shifted);
+
+            // Write out and read back in again
+            // Ensure that the changes were persisted
+            try (Workbook wb2 = _testDataProvider.writeOutAndReadBack(wb1)) {
+
+                sheet = wb2.getSheet("Sheet1");
+                assertEquals(4, sheet.getLastRowNum());
+
+                // Verify comments are in the position expected after the shift
+                assertNotNull(sheet.getCellComment(new CellAddress(0, 0)));
+                assertNull(sheet.getCellComment(new CellAddress(1, 0)));
+                assertNull(sheet.getCellComment(new CellAddress(2, 0)));
+                assertNotNull(sheet.getCellComment(new CellAddress(3, 0)));
+                assertNotNull(sheet.getCellComment(new CellAddress(4, 0)));
+
+                comment1_shifted = sheet.getCellComment(new CellAddress(0, 0)).getString().getString();
+                assertEquals(comment1, comment1_shifted);
+                comment3_shifted = sheet.getCellComment(new CellAddress(3, 0)).getString().getString();
+                assertEquals(comment3, comment3_shifted);
+                comment4_shifted = sheet.getCellComment(new CellAddress(4, 0)).getString().getString();
+                assertEquals(comment4, comment4_shifted);
+
+                // Shifting back up again, now two rows
+                sheet.shiftRows(2, sheet.getLastRowNum(), -2, true, true);
+
+                // TODO: it seems HSSFSheet does not correctly remove comments from rows that are overwritten
+                // by shifting rows...
+                if (!(wb2 instanceof HSSFWorkbook)) {
+                    assertEquals(2, sheet.getLastRowNum());
+
+                    // Verify comments are in the position expected
+                    assertNull(sheet.getCellComment(new CellAddress(0, 0)),
+                        "Had: " + (sheet.getCellComment(new CellAddress(0, 0)) == null ? "null" : sheet.getCellComment(new CellAddress(0, 0)).getString()));
+                    assertNotNull(sheet.getCellComment(new CellAddress(1, 0)));
+                    assertNotNull(sheet.getCellComment(new CellAddress(2, 0)));
+                }
+
+                comment1 = sheet.getCellComment(new CellAddress(1, 0)).getString().getString();
+                assertEquals(comment1, "comment top row3 (index2)\n");
+                String comment2 = sheet.getCellComment(new CellAddress(2, 0)).getString().getString();
+                assertEquals(comment2, "comment top row4 (index3)\n");
+            }
         }
-
-        comment1 = sheet.getCellComment(new CellAddress(1,0)).getString().getString();
-        assertEquals(comment1,"comment top row3 (index2)\n");
-        String comment2 = sheet.getCellComment(new CellAddress(2,0)).getString().getString();
-        assertEquals(comment2,"comment top row4 (index3)\n");
-
-        wb2.close();
     }
 
     @Test
     public final void testShiftWithNames() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet1 = wb.createSheet("Sheet1");
-        wb.createSheet("Sheet2");
-        Row row = sheet1.createRow(0);
-        row.createCell(0).setCellValue(1.1);
-        row.createCell(1).setCellValue(2.2);
-
-        Name name1 = wb.createName();
-        name1.setNameName("name1");
-        name1.setRefersToFormula("Sheet1!$A$1+Sheet1!$B$1");
-
-        Name name2 = wb.createName();
-        name2.setNameName("name2");
-        name2.setRefersToFormula("Sheet1!$A$1");
-
-        //refers to A1 but on Sheet2. Should stay unaffected.
-        Name name3 = wb.createName();
-        name3.setNameName("name3");
-        name3.setRefersToFormula("Sheet2!$A$1");
-
-        //The scope of this one is Sheet2. Should stay unaffected.
-        Name name4 = wb.createName();
-        name4.setNameName("name4");
-        name4.setRefersToFormula("A1");
-        name4.setSheetIndex(1);
-
-        sheet1.shiftRows(0, 1, 2);  //shift down the top row on Sheet1.
-        name1 = wb.getName("name1");
-        assertEquals("Sheet1!$A$3+Sheet1!$B$3", name1.getRefersToFormula());
-
-        name2 = wb.getName("name2");
-        assertEquals("Sheet1!$A$3", name2.getRefersToFormula());
-
-        //name3 and name4 refer to Sheet2 and should not be affected
-        name3 = wb.getName("name3");
-        assertEquals("Sheet2!$A$1", name3.getRefersToFormula());
-
-        name4 = wb.getName("name4");
-        assertEquals("A1", name4.getRefersToFormula());
-
-        wb.close();
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet1 = wb.createSheet("Sheet1");
+            wb.createSheet("Sheet2");
+            Row row = sheet1.createRow(0);
+            row.createCell(0).setCellValue(1.1);
+            row.createCell(1).setCellValue(2.2);
+
+            Name name1 = wb.createName();
+            name1.setNameName("name1");
+            name1.setRefersToFormula("Sheet1!$A$1+Sheet1!$B$1");
+
+            Name name2 = wb.createName();
+            name2.setNameName("name2");
+            name2.setRefersToFormula("Sheet1!$A$1");
+
+            //refers to A1 but on Sheet2. Should stay unaffected.
+            Name name3 = wb.createName();
+            name3.setNameName("name3");
+            name3.setRefersToFormula("Sheet2!$A$1");
+
+            //The scope of this one is Sheet2. Should stay unaffected.
+            Name name4 = wb.createName();
+            name4.setNameName("name4");
+            name4.setRefersToFormula("A1");
+            name4.setSheetIndex(1);
+
+            sheet1.shiftRows(0, 1, 2);  //shift down the top row on Sheet1.
+            name1 = wb.getName("name1");
+            assertEquals("Sheet1!$A$3+Sheet1!$B$3", name1.getRefersToFormula());
+
+            name2 = wb.getName("name2");
+            assertEquals("Sheet1!$A$3", name2.getRefersToFormula());
+
+            //name3 and name4 refer to Sheet2 and should not be affected
+            name3 = wb.getName("name3");
+            assertEquals("Sheet2!$A$1", name3.getRefersToFormula());
+
+            name4 = wb.getName("name4");
+            assertEquals("A1", name4.getRefersToFormula());
+        }
     }
 
     @Test
     public final void testShiftWithMergedRegions() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet();
-        Row row = sheet.createRow(0);
-        row.createCell(0).setCellValue(1.1);
-        row.createCell(1).setCellValue(2.2);
-        CellRangeAddress region = new CellRangeAddress(0, 0, 0, 2);
-        assertEquals("A1:C1", region.formatAsString());
-
-        assertEquals(0, sheet.addMergedRegion(region));
-
-        sheet.shiftRows(0, 1, 2);
-        region = sheet.getMergedRegion(0);
-        assertEquals("A3:C3", region.formatAsString());
-        wb.close();
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet();
+            Row row = sheet.createRow(0);
+            row.createCell(0).setCellValue(1.1);
+            row.createCell(1).setCellValue(2.2);
+            CellRangeAddress region = new CellRangeAddress(0, 0, 0, 2);
+            assertEquals("A1:C1", region.formatAsString());
+
+            assertEquals(0, sheet.addMergedRegion(region));
+
+            sheet.shiftRows(0, 1, 2);
+            region = sheet.getMergedRegion(0);
+            assertEquals("A3:C3", region.formatAsString());
+        }
     }
 
     //@Disabled("bug 56454: Incorrectly handles merged regions that do not contain column 0")
     @Test
     public final void shiftWithMergedRegions_bug56454() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet();
-        // populate sheet cells
-        for (int i = 0; i < 10; i++) {
-            Row row = sheet.createRow(i);
-
-            for (int j = 0; j < 10; j++) {
-                Cell cell = row.createCell(j, CellType.STRING);
-                cell.setCellValue(i + "x" + j);
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet();
+            // populate sheet cells
+            for (int i = 0; i < 10; i++) {
+                Row row = sheet.createRow(i);
+
+                for (int j = 0; j < 10; j++) {
+                    Cell cell = row.createCell(j, CellType.STRING);
+                    cell.setCellValue(i + "x" + j);
+                }
             }
-        }
 
-        CellRangeAddress A4_B7 = CellRangeAddress.valueOf("A4:B7");
-        CellRangeAddress C4_D7 = CellRangeAddress.valueOf("C4:D7");
+            CellRangeAddress A4_B7 = CellRangeAddress.valueOf("A4:B7");
+            CellRangeAddress C4_D7 = CellRangeAddress.valueOf("C4:D7");
 
-        assertEquals(0, sheet.addMergedRegion(A4_B7));
-        assertEquals(1, sheet.addMergedRegion(C4_D7));
+            assertEquals(0, sheet.addMergedRegion(A4_B7));
+            assertEquals(1, sheet.addMergedRegion(C4_D7));
 
-        assumeTrue(sheet.getLastRowNum() > 8);
+            assumeTrue(sheet.getLastRowNum() > 8);
 
-        // Insert a row in the middle of both merged regions.
-        sheet.shiftRows(4, sheet.getLastRowNum(), 1);
+            // Insert a row in the middle of both merged regions.
+            sheet.shiftRows(4, sheet.getLastRowNum(), 1);
 
-        // all regions should still start at row 3, and elongate by 1 row
-        List expectedMergedRegions = new ArrayList<>();
-        CellRangeAddress A4_B8 = CellRangeAddress.valueOf("A4:B8"); //A4:B7 should be elongated by 1 row
-        CellRangeAddress C4_D8 = CellRangeAddress.valueOf("C4:D8"); //C4:B7 should be elongated by 1 row
-        expectedMergedRegions.add(A4_B8);
-        expectedMergedRegions.add(C4_D8);
+            // all regions should still start at row 3, and elongate by 1 row
+            List expectedMergedRegions = new ArrayList<>();
+            CellRangeAddress A4_B8 = CellRangeAddress.valueOf("A4:B8"); //A4:B7 should be elongated by 1 row
+            CellRangeAddress C4_D8 = CellRangeAddress.valueOf("C4:D8"); //C4:B7 should be elongated by 1 row
+            expectedMergedRegions.add(A4_B8);
+            expectedMergedRegions.add(C4_D8);
 
-        // This test is written as expected-to-fail and should be rewritten
-        // as expected-to-pass when the bug is fixed.
-        // FIXME: remove try, catch, and testPassesNow, skipTest when test passes
-        try {
-            assertEquals(expectedMergedRegions, sheet.getMergedRegions());
-            testPassesNow(56454);
-        } catch (AssertionError e) {
-            skipTest(e);
+            // This test is written as expected-to-fail and should be rewritten
+            // as expected-to-pass when the bug is fixed.
+            // FIXME: remove try, catch, and testPassesNow, skipTest when test passes
+            assertNotEquals(expectedMergedRegions, sheet.getMergedRegions());
         }
-        wb.close();
     }
 
 
@@ -347,60 +336,59 @@ public abstract class BaseTestSheetShiftRows {
      */
     @Test
     public final void testShiftWithFormulas() throws IOException {
-        Workbook wb = _testDataProvider.openSampleWorkbook("ForShifting." + _testDataProvider.getStandardFileNameExtension());
+        try (Workbook wb = _testDataProvider.openSampleWorkbook("ForShifting." + _testDataProvider.getStandardFileNameExtension())) {
 
-        Sheet sheet = wb.getSheet("Sheet1");
-        assertEquals(20, sheet.getLastRowNum());
+            Sheet sheet = wb.getSheet("Sheet1");
+            assertEquals(20, sheet.getLastRowNum());
 
-        confirmRow(sheet, 0, 1, 171, 1, "ROW(D1)", "100+B1", "COUNT(D1:E1)");
-        confirmRow(sheet, 1, 2, 172, 1, "ROW(D2)", "100+B2", "COUNT(D2:E2)");
-        confirmRow(sheet, 2, 3, 173, 1, "ROW(D3)", "100+B3", "COUNT(D3:E3)");
+            confirmRow(sheet, 0, 1, 171, 1, "ROW(D1)", "100+B1", "COUNT(D1:E1)");
+            confirmRow(sheet, 1, 2, 172, 1, "ROW(D2)", "100+B2", "COUNT(D2:E2)");
+            confirmRow(sheet, 2, 3, 173, 1, "ROW(D3)", "100+B3", "COUNT(D3:E3)");
 
-        confirmCell(sheet, 6, 1, 271, "200+B1");
-        confirmCell(sheet, 7, 1, 272, "200+B2");
-        confirmCell(sheet, 8, 1, 273, "200+B3");
+            confirmCell(sheet, 6, 1, 271, "200+B1");
+            confirmCell(sheet, 7, 1, 272, "200+B2");
+            confirmCell(sheet, 8, 1, 273, "200+B3");
 
-        confirmCell(sheet, 14, 0, 0.0, "A12"); // the cell referred to by this formula will be replaced
+            confirmCell(sheet, 14, 0, 0.0, "A12"); // the cell referred to by this formula will be replaced
 
-        // -----------
-        // Row index 1 -> 11 (row "2" -> row "12")
-        sheet.shiftRows(1, 1, 10);
+            // -----------
+            // Row index 1 -> 11 (row "2" -> row "12")
+            sheet.shiftRows(1, 1, 10);
 
-        // Now check what sheet looks like after move
+            // Now check what sheet looks like after move
 
-        // no changes on row "1"
-        confirmRow(sheet, 0, 1, 171, 1, "ROW(D1)", "100+B1", "COUNT(D1:E1)");
+            // no changes on row "1"
+            confirmRow(sheet, 0, 1, 171, 1, "ROW(D1)", "100+B1", "COUNT(D1:E1)");
 
-        // row "2" is now empty
-        confirmEmptyRow(sheet, 1);
+            // row "2" is now empty
+            confirmEmptyRow(sheet, 1);
 
-        // Row "2" moved to row "12", and the formula has been updated.
-        // note however that the cached formula result (2) has not been updated. (POI differs from Excel here)
-        confirmRow(sheet, 11, 2, 172, 1, "ROW(D12)", "100+B12", "COUNT(D12:E12)");
+            // Row "2" moved to row "12", and the formula has been updated.
+            // note however that the cached formula result (2) has not been updated. (POI differs from Excel here)
+            confirmRow(sheet, 11, 2, 172, 1, "ROW(D12)", "100+B12", "COUNT(D12:E12)");
 
-        // no changes on row "3"
-        confirmRow(sheet, 2, 3, 173, 1, "ROW(D3)", "100+B3", "COUNT(D3:E3)");
+            // no changes on row "3"
+            confirmRow(sheet, 2, 3, 173, 1, "ROW(D3)", "100+B3", "COUNT(D3:E3)");
 
 
-        confirmCell(sheet, 14, 0, 0.0, "#REF!");
+            confirmCell(sheet, 14, 0, 0.0, "#REF!");
 
 
-        // Formulas on rows that weren't shifted:
-        confirmCell(sheet, 6, 1, 271, "200+B1");
-        confirmCell(sheet, 7, 1, 272, "200+B12"); // this one moved
-        confirmCell(sheet, 8, 1, 273, "200+B3");
+            // Formulas on rows that weren't shifted:
+            confirmCell(sheet, 6, 1, 271, "200+B1");
+            confirmCell(sheet, 7, 1, 272, "200+B12"); // this one moved
+            confirmCell(sheet, 8, 1, 273, "200+B3");
 
-        // check formulas on other sheets
-        Sheet sheet2 = wb.getSheet("Sheet2");
-        confirmCell(sheet2,  0, 0, 371, "300+Sheet1!B1");
-        confirmCell(sheet2,  1, 0, 372, "300+Sheet1!B12");
-        confirmCell(sheet2,  2, 0, 373, "300+Sheet1!B3");
+            // check formulas on other sheets
+            Sheet sheet2 = wb.getSheet("Sheet2");
+            confirmCell(sheet2, 0, 0, 371, "300+Sheet1!B1");
+            confirmCell(sheet2, 1, 0, 372, "300+Sheet1!B12");
+            confirmCell(sheet2, 2, 0, 373, "300+Sheet1!B3");
 
-        confirmCell(sheet2, 11, 0, 300, "300+Sheet1!#REF!");
+            confirmCell(sheet2, 11, 0, 300, "300+Sheet1!#REF!");
 
-
-        // Note - named ranges formulas have not been updated
-        wb.close();
+            // Note - named ranges formulas have not been updated
+        }
     }
 
     private static void confirmRow(Sheet sheet, int rowIx, double valA, double valB, double valC,
@@ -419,72 +407,72 @@ public abstract class BaseTestSheetShiftRows {
 
     @Test
     public final void testShiftSharedFormulasBug54206() throws IOException {
-        Workbook wb = _testDataProvider.openSampleWorkbook("54206." + _testDataProvider.getStandardFileNameExtension());
-
-        Sheet sheet = wb.getSheetAt(0);
-        assertEquals("SUMIF($B$19:$B$82,$B4,G$19:G$82)", sheet.getRow(3).getCell(6).getCellFormula());
-        assertEquals("SUMIF($B$19:$B$82,$B4,H$19:H$82)", sheet.getRow(3).getCell(7).getCellFormula());
-        assertEquals("SUMIF($B$19:$B$82,$B4,I$19:I$82)", sheet.getRow(3).getCell(8).getCellFormula());
-
-        assertEquals("SUMIF($B$19:$B$82,$B15,G$19:G$82)", sheet.getRow(14).getCell(6).getCellFormula());
-        assertEquals("SUMIF($B$19:$B$82,$B15,H$19:H$82)", sheet.getRow(14).getCell(7).getCellFormula());
-        assertEquals("SUMIF($B$19:$B$82,$B15,I$19:I$82)", sheet.getRow(14).getCell(8).getCellFormula());
-
-        // now the whole block G4L:15
-        for(int i = 3; i <= 14; i++){
-            for(int j = 6; j <= 8; j++){
-                String col = CellReference.convertNumToColString(j);
-                String expectedFormula = "SUMIF($B$19:$B$82,$B"+(i+1)+","+col+"$19:"+col+"$82)";
-                assertEquals(expectedFormula, sheet.getRow(i).getCell(j).getCellFormula());
+        try (Workbook wb = _testDataProvider.openSampleWorkbook("54206." + _testDataProvider.getStandardFileNameExtension())) {
+
+            Sheet sheet = wb.getSheetAt(0);
+            assertEquals("SUMIF($B$19:$B$82,$B4,G$19:G$82)", sheet.getRow(3).getCell(6).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$82,$B4,H$19:H$82)", sheet.getRow(3).getCell(7).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$82,$B4,I$19:I$82)", sheet.getRow(3).getCell(8).getCellFormula());
+
+            assertEquals("SUMIF($B$19:$B$82,$B15,G$19:G$82)", sheet.getRow(14).getCell(6).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$82,$B15,H$19:H$82)", sheet.getRow(14).getCell(7).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$82,$B15,I$19:I$82)", sheet.getRow(14).getCell(8).getCellFormula());
+
+            // now the whole block G4L:15
+            for (int i = 3; i <= 14; i++) {
+                for (int j = 6; j <= 8; j++) {
+                    String col = CellReference.convertNumToColString(j);
+                    String expectedFormula = "SUMIF($B$19:$B$82,$B" + (i + 1) + "," + col + "$19:" + col + "$82)";
+                    assertEquals(expectedFormula, sheet.getRow(i).getCell(j).getCellFormula());
+                }
             }
-        }
 
-        assertEquals("SUM(G24:I24)", sheet.getRow(23).getCell(9).getCellFormula());
-        assertEquals("SUM(G25:I25)", sheet.getRow(24).getCell(9).getCellFormula());
-        assertEquals("SUM(G26:I26)", sheet.getRow(25).getCell(9).getCellFormula());
+            assertEquals("SUM(G24:I24)", sheet.getRow(23).getCell(9).getCellFormula());
+            assertEquals("SUM(G25:I25)", sheet.getRow(24).getCell(9).getCellFormula());
+            assertEquals("SUM(G26:I26)", sheet.getRow(25).getCell(9).getCellFormula());
 
-        sheet.shiftRows(24, sheet.getLastRowNum(), 4, true, false);
+            sheet.shiftRows(24, sheet.getLastRowNum(), 4, true, false);
 
-        assertEquals("SUMIF($B$19:$B$86,$B4,G$19:G$86)", sheet.getRow(3).getCell(6).getCellFormula());
-        assertEquals("SUMIF($B$19:$B$86,$B4,H$19:H$86)", sheet.getRow(3).getCell(7).getCellFormula());
-        assertEquals("SUMIF($B$19:$B$86,$B4,I$19:I$86)", sheet.getRow(3).getCell(8).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$86,$B4,G$19:G$86)", sheet.getRow(3).getCell(6).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$86,$B4,H$19:H$86)", sheet.getRow(3).getCell(7).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$86,$B4,I$19:I$86)", sheet.getRow(3).getCell(8).getCellFormula());
 
-        assertEquals("SUMIF($B$19:$B$86,$B15,G$19:G$86)", sheet.getRow(14).getCell(6).getCellFormula());
-        assertEquals("SUMIF($B$19:$B$86,$B15,H$19:H$86)", sheet.getRow(14).getCell(7).getCellFormula());
-        assertEquals("SUMIF($B$19:$B$86,$B15,I$19:I$86)", sheet.getRow(14).getCell(8).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$86,$B15,G$19:G$86)", sheet.getRow(14).getCell(6).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$86,$B15,H$19:H$86)", sheet.getRow(14).getCell(7).getCellFormula());
+            assertEquals("SUMIF($B$19:$B$86,$B15,I$19:I$86)", sheet.getRow(14).getCell(8).getCellFormula());
 
-        // now the whole block G4L:15
-        for(int i = 3; i <= 14; i++){
-            for(int j = 6; j <= 8; j++){
-                String col = CellReference.convertNumToColString(j);
-                String expectedFormula = "SUMIF($B$19:$B$86,$B"+(i+1)+","+col+"$19:"+col+"$86)";
-                assertEquals(expectedFormula, sheet.getRow(i).getCell(j).getCellFormula());
+            // now the whole block G4L:15
+            for (int i = 3; i <= 14; i++) {
+                for (int j = 6; j <= 8; j++) {
+                    String col = CellReference.convertNumToColString(j);
+                    String expectedFormula = "SUMIF($B$19:$B$86,$B" + (i + 1) + "," + col + "$19:" + col + "$86)";
+                    assertEquals(expectedFormula, sheet.getRow(i).getCell(j).getCellFormula());
+                }
             }
-        }
 
-        assertEquals("SUM(G24:I24)", sheet.getRow(23).getCell(9).getCellFormula());
+            assertEquals("SUM(G24:I24)", sheet.getRow(23).getCell(9).getCellFormula());
 
-        // shifted rows
-        assertTrue( sheet.getRow(24) == null || sheet.getRow(24).getCell(9) == null);
-        assertTrue( sheet.getRow(25) == null || sheet.getRow(25).getCell(9) == null);
-        assertTrue( sheet.getRow(26) == null || sheet.getRow(26).getCell(9) == null);
-        assertTrue( sheet.getRow(27) == null || sheet.getRow(27).getCell(9) == null);
+            // shifted rows
+            assertTrue(sheet.getRow(24) == null || sheet.getRow(24).getCell(9) == null);
+            assertTrue(sheet.getRow(25) == null || sheet.getRow(25).getCell(9) == null);
+            assertTrue(sheet.getRow(26) == null || sheet.getRow(26).getCell(9) == null);
+            assertTrue(sheet.getRow(27) == null || sheet.getRow(27).getCell(9) == null);
 
-        assertEquals("SUM(G29:I29)", sheet.getRow(28).getCell(9).getCellFormula());
-        assertEquals("SUM(G30:I30)", sheet.getRow(29).getCell(9).getCellFormula());
-        wb.close();
+            assertEquals("SUM(G29:I29)", sheet.getRow(28).getCell(9).getCellFormula());
+            assertEquals("SUM(G30:I30)", sheet.getRow(29).getCell(9).getCellFormula());
+        }
     }
 
     @Test
     void testBug55280() throws IOException {
-        Workbook w = _testDataProvider.createWorkbook();
-        Sheet s = w.createSheet();
-        for (int row = 0; row < 5000; ++row) {
-            assertEquals(row, s.addMergedRegion(new CellRangeAddress(row, row, 0, 3)));
-        }
+        try (Workbook w = _testDataProvider.createWorkbook()) {
+            Sheet s = w.createSheet();
+            for (int row = 0; row < 5000; ++row) {
+                assertEquals(row, s.addMergedRegion(new CellRangeAddress(row, row, 0, 3)));
+            }
 
-        s.shiftRows(0, 4999, 1);        // takes a long time...
-        w.close();
+            s.shiftRows(0, 4999, 1);        // takes a long time...
+        }
     }
 
     @Test
@@ -504,203 +492,191 @@ public abstract class BaseTestSheetShiftRows {
      */
     @Test
     void testBug46742_52903_shiftHyperlinks() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet("test");
-        Row row = sheet.createRow(0);
-
-        // How to create hyperlinks
-        // https://poi.apache.org/spreadsheet/quick-guide.html#Hyperlinks
-        CreationHelper helper = wb.getCreationHelper();
-        CellStyle hlinkStyle = wb.createCellStyle();
-        Font hlinkFont = wb.createFont();
-        hlinkFont.setUnderline(Font.U_SINGLE);
-        hlinkFont.setColor(IndexedColors.BLUE.getIndex());
-        hlinkStyle.setFont(hlinkFont);
-
-        // 3D relative document link
-        // CellAddress=A1, shifted to A4
-        Cell cell = row.createCell(0);
-        cell.setCellStyle(hlinkStyle);
-        createHyperlink(helper, cell, HyperlinkType.DOCUMENT, "test!E1");
-
-        // URL
-        cell = row.createCell(1);
-        // CellAddress=B1, shifted to B4
-        cell.setCellStyle(hlinkStyle);
-        createHyperlink(helper, cell, HyperlinkType.URL, "https://poi.apache.org/");
-
-        // row0 will be shifted on top of row1, so this URL should be removed from the workbook
-        Row overwrittenRow = sheet.createRow(3);
-        cell = overwrittenRow.createCell(2);
-        // CellAddress=C4, will be overwritten (deleted)
-        cell.setCellStyle(hlinkStyle);
-        createHyperlink(helper, cell, HyperlinkType.EMAIL, "mailto:poi@apache.org");
-
-        // hyperlinks on this row are unaffected by the row shifting, so the hyperlinks should not move
-        Row unaffectedRow = sheet.createRow(20);
-        cell = unaffectedRow.createCell(3);
-        // CellAddress=D21, will be unaffected
-        cell.setCellStyle(hlinkStyle);
-        createHyperlink(helper, cell, HyperlinkType.FILE, "54524.xlsx");
-
-        cell = wb.createSheet("other").createRow(0).createCell(0);
-        // CellAddress=Other!A1, will be unaffected
-        cell.setCellStyle(hlinkStyle);
-        createHyperlink(helper, cell, HyperlinkType.URL, "http://apache.org/");
-
-        int startRow = 0;
-        int endRow = 0;
-        int n = 3;
-        sheet.shiftRows(startRow, endRow, n);
-
-        Workbook read = _testDataProvider.writeOutAndReadBack(wb);
-        wb.close();
-
-        Sheet sh = read.getSheet("test");
-
-        Row shiftedRow = sh.getRow(3);
-
-        // document link anchored on a shifted cell should be moved
-        // Note that hyperlinks do not track what they point to, so this hyperlink should still refer to test!E1
-        verifyHyperlink(shiftedRow.getCell(0), HyperlinkType.DOCUMENT, "test!E1");
-
-        // URL, EMAIL, and FILE links anchored on a shifted cell should be moved
-        verifyHyperlink(shiftedRow.getCell(1), HyperlinkType.URL, "https://poi.apache.org/");
-
-        // Make sure hyperlinks were moved and not copied
-        assertNull(sh.getHyperlink(0, 0), "Document hyperlink should be moved, not copied");
-        assertNull(sh.getHyperlink(0, 1), "URL hyperlink should be moved, not copied");
-
-        // Make sure hyperlink in overwritten row is deleted
-        assertEquals(3, sh.getHyperlinkList().size());
-        CellAddress unexpectedLinkAddress = new CellAddress("C4");
-        for (Hyperlink link : sh.getHyperlinkList()) {
-            final CellAddress linkAddress = new CellAddress(link.getFirstRow(), link.getFirstColumn());
-            assertNotEquals(linkAddress, unexpectedLinkAddress,
-                "Row 4, including the hyperlink at C4, should have " +
-                    "been deleted when Row 1 was shifted on top of it.");
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet("test");
+            Row row = sheet.createRow(0);
+
+            // How to create hyperlinks
+            // https://poi.apache.org/spreadsheet/quick-guide.html#Hyperlinks
+            CreationHelper helper = wb.getCreationHelper();
+            CellStyle hlinkStyle = wb.createCellStyle();
+            Font hlinkFont = wb.createFont();
+            hlinkFont.setUnderline(Font.U_SINGLE);
+            hlinkFont.setColor(IndexedColors.BLUE.getIndex());
+            hlinkStyle.setFont(hlinkFont);
+
+            // 3D relative document link
+            // CellAddress=A1, shifted to A4
+            Cell cell = row.createCell(0);
+            cell.setCellStyle(hlinkStyle);
+            createHyperlink(helper, cell, HyperlinkType.DOCUMENT, "test!E1");
+
+            // URL
+            cell = row.createCell(1);
+            // CellAddress=B1, shifted to B4
+            cell.setCellStyle(hlinkStyle);
+            createHyperlink(helper, cell, HyperlinkType.URL, "https://poi.apache.org/");
+
+            // row0 will be shifted on top of row1, so this URL should be removed from the workbook
+            Row overwrittenRow = sheet.createRow(3);
+            cell = overwrittenRow.createCell(2);
+            // CellAddress=C4, will be overwritten (deleted)
+            cell.setCellStyle(hlinkStyle);
+            createHyperlink(helper, cell, HyperlinkType.EMAIL, "mailto:poi@apache.org");
+
+            // hyperlinks on this row are unaffected by the row shifting, so the hyperlinks should not move
+            Row unaffectedRow = sheet.createRow(20);
+            cell = unaffectedRow.createCell(3);
+            // CellAddress=D21, will be unaffected
+            cell.setCellStyle(hlinkStyle);
+            createHyperlink(helper, cell, HyperlinkType.FILE, "54524.xlsx");
+
+            cell = wb.createSheet("other").createRow(0).createCell(0);
+            // CellAddress=Other!A1, will be unaffected
+            cell.setCellStyle(hlinkStyle);
+            createHyperlink(helper, cell, HyperlinkType.URL, "http://apache.org/");
+
+            int startRow = 0;
+            int endRow = 0;
+            int n = 3;
+            sheet.shiftRows(startRow, endRow, n);
+
+            try (Workbook read = _testDataProvider.writeOutAndReadBack(wb)) {
+                Sheet sh = read.getSheet("test");
+                Row shiftedRow = sh.getRow(3);
+
+                // document link anchored on a shifted cell should be moved
+                // Note that hyperlinks do not track what they point to, so this hyperlink should still refer to test!E1
+                verifyHyperlink(shiftedRow.getCell(0), HyperlinkType.DOCUMENT, "test!E1");
+
+                // URL, EMAIL, and FILE links anchored on a shifted cell should be moved
+                verifyHyperlink(shiftedRow.getCell(1), HyperlinkType.URL, "https://poi.apache.org/");
+
+                // Make sure hyperlinks were moved and not copied
+                assertNull(sh.getHyperlink(0, 0), "Document hyperlink should be moved, not copied");
+                assertNull(sh.getHyperlink(0, 1), "URL hyperlink should be moved, not copied");
+
+                // Make sure hyperlink in overwritten row is deleted
+                assertEquals(3, sh.getHyperlinkList().size());
+                CellAddress unexpectedLinkAddress = new CellAddress("C4");
+                for (Hyperlink link : sh.getHyperlinkList()) {
+                    final CellAddress linkAddress = new CellAddress(link.getFirstRow(), link.getFirstColumn());
+                    assertNotEquals(linkAddress, unexpectedLinkAddress,
+                        "Row 4, including the hyperlink at C4, should have " +
+                            "been deleted when Row 1 was shifted on top of it.");
+                }
+
+                // Make sure unaffected rows are not shifted
+                Cell unaffectedCell = sh.getRow(20).getCell(3);
+                assertTrue(cellHasHyperlink(unaffectedCell));
+                verifyHyperlink(unaffectedCell, HyperlinkType.FILE, "54524.xlsx");
+
+                // Make sure cells on other sheets are not affected
+                unaffectedCell = read.getSheet("other").getRow(0).getCell(0);
+                assertTrue(cellHasHyperlink(unaffectedCell));
+                verifyHyperlink(unaffectedCell, HyperlinkType.URL, "http://apache.org/");
+            }
         }
-
-        // Make sure unaffected rows are not shifted
-        Cell unaffectedCell = sh.getRow(20).getCell(3);
-        assertTrue(cellHasHyperlink(unaffectedCell));
-        verifyHyperlink(unaffectedCell, HyperlinkType.FILE, "54524.xlsx");
-
-        // Make sure cells on other sheets are not affected
-        unaffectedCell = read.getSheet("other").getRow(0).getCell(0);
-        assertTrue(cellHasHyperlink(unaffectedCell));
-        verifyHyperlink(unaffectedCell, HyperlinkType.URL, "http://apache.org/");
-
-        read.close();
     }
 
     //@Disabled("bug 56454: Incorrectly handles merged regions that do not contain column 0")
     @Test
     void shiftRowsWithMergedRegionsThatDoNotContainColumnZero() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet("test");
-
-        // populate sheet cells
-        for (int i = 0; i < 10; i++) {
-            Row row = sheet.createRow(i);
-            for (int j = 0; j < 12; j++) {
-                Cell cell = row.createCell(j);
-                cell.setCellValue(i + "x" + j);
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet("test");
+
+            // populate sheet cells
+            for (int i = 0; i < 10; i++) {
+                Row row = sheet.createRow(i);
+                for (int j = 0; j < 12; j++) {
+                    Cell cell = row.createCell(j);
+                    cell.setCellValue(i + "x" + j);
+                }
             }
-        }
 
-        CellRangeAddress A4_B7 = new CellRangeAddress(3, 6, 0, 1);
-        CellRangeAddress C5_D7 = new CellRangeAddress(4, 6, 2, 3);
+            CellRangeAddress A4_B7 = new CellRangeAddress(3, 6, 0, 1);
+            CellRangeAddress C5_D7 = new CellRangeAddress(4, 6, 2, 3);
 
-        assertEquals(0, sheet.addMergedRegion(A4_B7));
-        assertEquals(1, sheet.addMergedRegion(C5_D7));
+            assertEquals(0, sheet.addMergedRegion(A4_B7));
+            assertEquals(1, sheet.addMergedRegion(C5_D7));
 
-        // A4:B7 will elongate vertically
-        // C5:D7 will be shifted down with same size
-        sheet.shiftRows(4, sheet.getLastRowNum(), 1);
+            // A4:B7 will elongate vertically
+            // C5:D7 will be shifted down with same size
+            sheet.shiftRows(4, sheet.getLastRowNum(), 1);
 
-        // This test is written as expected-to-fail and should be rewritten
-        // as expected-to-pass when the bug is fixed.
-        // FIXME: remove try, catch, and testPassesNow, skipTest when test passes
-        try {
+            // This test is written as expected-to-fail and should be rewritten
+            // as expected-to-pass when the bug is fixed.
+            // FIXME: remove try, catch, and testPassesNow, skipTest when test passes
             assertEquals(2, sheet.getNumMergedRegions());
-            assertEquals(CellRangeAddress.valueOf("A4:B8"), sheet.getMergedRegion(0));
-            assertEquals(CellRangeAddress.valueOf("C5:D8"), sheet.getMergedRegion(1));
-            testPassesNow(56454);
-        } catch (AssertionError e) {
-            skipTest(e);
+            assertNotEquals(CellRangeAddress.valueOf("A4:B8"), sheet.getMergedRegion(0));
+            assertNotEquals(CellRangeAddress.valueOf("C5:D8"), sheet.getMergedRegion(1));
         }
-
-        wb.close();
     }
 
     @Test
     void shiftMergedRowsToMergedRowsUp() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet("test");
-        populateSheetCells(sheet, 2);
-
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet("test");
+            populateSheetCells(sheet, 2);
 
-        CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4);
-        CellRangeAddress A2_C2 = new CellRangeAddress(1, 1, 0, 2);
 
-        assertEquals(0, sheet.addMergedRegion(A1_E1));
-        assertEquals(1, sheet.addMergedRegion(A2_C2));
+            CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4);
+            CellRangeAddress A2_C2 = new CellRangeAddress(1, 1, 0, 2);
 
-        // A1:E1 should be removed
-        // A2:C2 will be A1:C1
-        sheet.shiftRows(1, sheet.getLastRowNum(), -1);
+            assertEquals(0, sheet.addMergedRegion(A1_E1));
+            assertEquals(1, sheet.addMergedRegion(A2_C2));
 
-        assertEquals(1, sheet.getNumMergedRegions());
-        assertEquals(CellRangeAddress.valueOf("A1:C1"), sheet.getMergedRegion(0));
+            // A1:E1 should be removed
+            // A2:C2 will be A1:C1
+            sheet.shiftRows(1, sheet.getLastRowNum(), -1);
 
-        wb.close();
+            assertEquals(1, sheet.getNumMergedRegions());
+            assertEquals(CellRangeAddress.valueOf("A1:C1"), sheet.getMergedRegion(0));
+        }
     }
 
     @Test
     void shiftMergedRowsToMergedRowsOverlappingMergedRegion() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet("test");
-        populateSheetCells(sheet, 10);
-
-        CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4);
-        CellRangeAddress A2_C2 = new CellRangeAddress(1, 7, 0, 2);
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet("test");
+            populateSheetCells(sheet, 10);
 
-        assertEquals(0, sheet.addMergedRegion(A1_E1));
-        assertEquals(1, sheet.addMergedRegion(A2_C2));
+            CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4);
+            CellRangeAddress A2_C2 = new CellRangeAddress(1, 7, 0, 2);
 
-        // A1:E1 should move to A5:E5
-        // A2:C2 should be removed
-        sheet.shiftRows(0, 0, 4);
+            assertEquals(0, sheet.addMergedRegion(A1_E1));
+            assertEquals(1, sheet.addMergedRegion(A2_C2));
 
-        assertEquals(1, sheet.getNumMergedRegions());
-        assertEquals(CellRangeAddress.valueOf("A5:E5"), sheet.getMergedRegion(0));
+            // A1:E1 should move to A5:E5
+            // A2:C2 should be removed
+            sheet.shiftRows(0, 0, 4);
 
-        wb.close();
+            assertEquals(1, sheet.getNumMergedRegions());
+            assertEquals(CellRangeAddress.valueOf("A5:E5"), sheet.getMergedRegion(0));
+        }
     }
 
     @Test
     void bug60384ShiftMergedRegion() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet("test");
-        populateSheetCells(sheet, 9);
-
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet("test");
+            populateSheetCells(sheet, 9);
 
-        CellRangeAddress A8_E8 = new CellRangeAddress(7, 7, 0, 4);
-        CellRangeAddress A9_C9 = new CellRangeAddress(8, 8, 0, 2);
 
-        assertEquals(0, sheet.addMergedRegion(A8_E8));
-        assertEquals(1, sheet.addMergedRegion(A9_C9));
+            CellRangeAddress A8_E8 = new CellRangeAddress(7, 7, 0, 4);
+            CellRangeAddress A9_C9 = new CellRangeAddress(8, 8, 0, 2);
 
-        // A1:E1 should be removed
-        // A2:C2 will be A1:C1
-        sheet.shiftRows(3, sheet.getLastRowNum(), 1);
+            assertEquals(0, sheet.addMergedRegion(A8_E8));
+            assertEquals(1, sheet.addMergedRegion(A9_C9));
 
-        assertEquals(2, sheet.getNumMergedRegions());
-        assertEquals(CellRangeAddress.valueOf("A9:E9"), sheet.getMergedRegion(0));
-        assertEquals(CellRangeAddress.valueOf("A10:C10"), sheet.getMergedRegion(1));
+            // A1:E1 should be removed
+            // A2:C2 will be A1:C1
+            sheet.shiftRows(3, sheet.getLastRowNum(), 1);
 
-        wb.close();
+            assertEquals(2, sheet.getNumMergedRegions());
+            assertEquals(CellRangeAddress.valueOf("A9:E9"), sheet.getMergedRegion(0));
+            assertEquals(CellRangeAddress.valueOf("A10:C10"), sheet.getMergedRegion(1));
+        }
     }
 
     private void populateSheetCells(Sheet sheet, int rowCount) {
@@ -716,80 +692,79 @@ public abstract class BaseTestSheetShiftRows {
 
     @Test
     void shiftMergedRowsToMergedRowsDown() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet("test");
-
-        // populate sheet cells
-        populateSheetCells(sheet, 2);
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet("test");
 
-        CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4);
-        CellRangeAddress A2_C2 = new CellRangeAddress(1, 1, 0, 2);
+            // populate sheet cells
+            populateSheetCells(sheet, 2);
 
-        assertEquals(0, sheet.addMergedRegion(A1_E1));
-        assertEquals(1, sheet.addMergedRegion(A2_C2));
+            CellRangeAddress A1_E1 = new CellRangeAddress(0, 0, 0, 4);
+            CellRangeAddress A2_C2 = new CellRangeAddress(1, 1, 0, 2);
 
-        // A1:E1 should be moved to A2:E2
-        // A2:C2 will be removed
-        sheet.shiftRows(0, 0, 1);
+            assertEquals(0, sheet.addMergedRegion(A1_E1));
+            assertEquals(1, sheet.addMergedRegion(A2_C2));
 
-        assertEquals(1, sheet.getNumMergedRegions());
-        assertEquals(CellRangeAddress.valueOf("A2:E2"), sheet.getMergedRegion(0));
+            // A1:E1 should be moved to A2:E2
+            // A2:C2 will be removed
+            sheet.shiftRows(0, 0, 1);
 
-        wb.close();
+            assertEquals(1, sheet.getNumMergedRegions());
+            assertEquals(CellRangeAddress.valueOf("A2:E2"), sheet.getMergedRegion(0));
+        }
     }
 
     @Test
     void test61840_shifting_rows_up_does_not_produce_REF_errors() throws IOException {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet();
-        Cell cell = sheet.createRow(4).createCell(0);
-
-        cell.setCellFormula("(B5-C5)/B5");
-        sheet.shiftRows(4, 4, -1);
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet();
+            Cell cell = sheet.createRow(4).createCell(0);
 
-        // Cell objects created before a row shift are still valid.
-        // The row number of those cell references will be shifted if
-        // the cell is within the shift range.
-        assertEquals("(B4-C4)/B4", cell.getCellFormula());
+            cell.setCellFormula("(B5-C5)/B5");
+            sheet.shiftRows(4, 4, -1);
 
-        // New cell references are also valid.
-        Cell shiftedCell = sheet.getRow(3).getCell(0);
-        assertNotNull(shiftedCell);
-        assertEquals("(B4-C4)/B4", shiftedCell.getCellFormula());
+            // Cell objects created before a row shift are still valid.
+            // The row number of those cell references will be shifted if
+            // the cell is within the shift range.
+            assertEquals("(B4-C4)/B4", cell.getCellFormula());
 
-        wb.close();
+            // New cell references are also valid.
+            Cell shiftedCell = sheet.getRow(3).getCell(0);
+            assertNotNull(shiftedCell);
+            assertEquals("(B4-C4)/B4", shiftedCell.getCellFormula());
+        }
     }
 
 
     @Test
-    void checkMergedRegions56454() {
-        Workbook wb = _testDataProvider.createWorkbook();
-        Sheet sheet = wb.createSheet();
+    void checkMergedRegions56454() throws IOException {
+        try (Workbook wb = _testDataProvider.createWorkbook()) {
+            Sheet sheet = wb.createSheet();
 
-        // populate sheet cells
-        for (int i = 0; i < 10; i++) {
-            Row row = sheet.createRow(i);
+            // populate sheet cells
+            for (int i = 0; i < 10; i++) {
+                Row row = sheet.createRow(i);
 
-            for (int j = 0; j < 10; j++) {
-                Cell cell = row.createCell(j, CellType.STRING);
+                for (int j = 0; j < 10; j++) {
+                    Cell cell = row.createCell(j, CellType.STRING);
 
-                cell.setCellValue(i + "x" + j);
+                    cell.setCellValue(i + "x" + j);
+                }
             }
-        }
 
-        CellRangeAddress region1 = new CellRangeAddress(3, 6, 0, 1);
-        CellRangeAddress region2 = new CellRangeAddress(3, 6, 2, 3);
+            CellRangeAddress region1 = new CellRangeAddress(3, 6, 0, 1);
+            CellRangeAddress region2 = new CellRangeAddress(3, 6, 2, 3);
 
-        sheet.addMergedRegion(region1);
-        sheet.addMergedRegion(region2);
+            sheet.addMergedRegion(region1);
+            sheet.addMergedRegion(region2);
 
-        sheet.shiftRows(4, sheet.getLastRowNum(), 1);
+            sheet.shiftRows(4, sheet.getLastRowNum(), 1);
 
-        // check, if all regions still start at row 3
-        for (int i = 0; i < sheet.getNumMergedRegions(); i++) {
-            CellRangeAddress cr = sheet.getMergedRegion(i);
+            // check, if all regions still start at row 3
+            for (int i = 0; i < sheet.getNumMergedRegions(); i++) {
+                CellRangeAddress cr = sheet.getMergedRegion(i);
 
-            assertEquals(cr.getFirstRow(), 3);
+                assertEquals(cr.getFirstRow(), 3);
+            }
         }
     }
 
diff --git a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestWorkbook.java b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
index 22b8dff2ab..150622eccc 100644
--- a/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
+++ b/poi/src/test/java/org/apache/poi/ss/usermodel/BaseTestWorkbook.java
@@ -724,7 +724,7 @@ public abstract class BaseTestWorkbook {
     }
 
     @Test
-    void changeSheetNameWithSharedFormulas() throws IOException {
+    protected void changeSheetNameWithSharedFormulas() throws IOException {
         String sampleFile = "shared_formulas.xls" + (getClass().getName().contains("xssf") ? "x" : "");
 
         try (Workbook wb = _testDataProvider.openSampleWorkbook(sampleFile)) {
-- 
cgit v1.2.3