From: Andreas Beeker Date: Sat, 16 Jan 2021 15:51:00 +0000 (+0000) Subject: integration tests: Fix handling of NullPointerExceptions for Java 16+ (again ...) X-Git-Tag: REL_5_1_0~424 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=44efecf42ed62aba1d6c9e149c3683bfd19f6453;p=poi.git integration tests: Fix handling of NullPointerExceptions for Java 16+ (again ...) Refactor TestAllFiles to provide an API for mass testing git-svn-id: https://svn.apache.org/repos/asf/poi/trunk@1885576 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/src/integrationtest/org/apache/poi/stress/ExcInfo.java b/src/integrationtest/org/apache/poi/stress/ExcInfo.java new file mode 100644 index 0000000000..c9241cb0f7 --- /dev/null +++ b/src/integrationtest/org/apache/poi/stress/ExcInfo.java @@ -0,0 +1,96 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ +package org.apache.poi.stress; + +import static org.junit.jupiter.api.Assertions.fail; + +public class ExcInfo { + private static final String IGNORED_TESTS = "IGNORE"; + + private String file; + private String tests; + private String handler; + private String password; + private Class exClazz; + private String exMessage; + + public String getFile() { + return file; + } + + public void setFile(String file) { + this.file = file; + } + + public String getTests() { + return tests; + } + + public void setTests(String tests) { + this.tests = tests; + } + + public String getHandler() { + return handler; + } + + public void setHandler(String handler) { + this.handler = handler; + } + + public String getPassword() { + return password; + } + + public void setPassword(String password) { + this.password = password; + } + + public Class getExClazz() { + return exClazz; + } + + @SuppressWarnings("unchecked") + public void setExClazz(String exClazz) { + try { + this.exClazz = (Class) Class.forName(exClazz); + } catch (ClassNotFoundException ex) { + fail(ex); + } + } + + public String getExMessage() { + return exMessage; + } + + public void setExMessage(String exMessage) { + this.exMessage = exMessage; + } + + public boolean isMatch(String testName, String handler) { + return + (tests == null || tests.contains(testName) || IGNORED_TESTS.equals(tests)) && + (this.handler == null || this.handler.contains(handler)); + } + + public boolean isValid(String testName, String handler) { + return + !IGNORED_TESTS.equals(tests) && + (tests == null || tests.contains(testName)) && + (this.handler == null || this.handler.contains(handler)); + } +} diff --git a/src/integrationtest/org/apache/poi/stress/FileHandlerKnown.java b/src/integrationtest/org/apache/poi/stress/FileHandlerKnown.java new file mode 100644 index 0000000000..aa3c827f1f --- /dev/null +++ b/src/integrationtest/org/apache/poi/stress/FileHandlerKnown.java @@ -0,0 +1,60 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ +package org.apache.poi.stress; + +import java.io.File; +import java.io.InputStream; +import java.util.function.Supplier; + +@SuppressWarnings("unused") +public enum FileHandlerKnown { + HDGF(HDGFFileHandler::new), + HMEF(HMEFFileHandler::new), + HPBF(HPBFFileHandler::new), + HPSF(HPSFFileHandler::new), + HSLF(HSLFFileHandler::new), + HSMF(HSMFFileHandler::new), + HSSF(HSSFFileHandler::new), + HWPF(HWPFFileHandler::new), + OPC(OPCFileHandler::new), + POIFS(POIFSFileHandler::new), + XDGF(XDGFFileHandler::new), + XSLF(XSLFFileHandler::new), + XSSFB(XSSFBFileHandler::new), + XSSF(XSSFFileHandler::new), + XWPF(XWPFFileHandler::new), + OWPF(OWPFFileHandler::new), + NULL(NullFileHandler::new) + ; + + public final Supplier fileHandler; + + FileHandlerKnown(Supplier fileHandler) { + this.fileHandler = fileHandler; + } + + private static class NullFileHandler implements FileHandler { + @Override + public void handleFile(InputStream stream, String path) {} + + @Override + public void handleExtracting(File file) {} + + @Override + public void handleAdditional(File file) {} + } +} diff --git a/src/integrationtest/org/apache/poi/stress/StressMap.java b/src/integrationtest/org/apache/poi/stress/StressMap.java new file mode 100644 index 0000000000..73d39b8530 --- /dev/null +++ b/src/integrationtest/org/apache/poi/stress/StressMap.java @@ -0,0 +1,150 @@ +/* ==================================================================== + Licensed to the Apache Software Foundation (ASF) under one or more + contributor license agreements. See the NOTICE file distributed with + this work for additional information regarding copyright ownership. + The ASF licenses this file to You under the Apache License, Version 2.0 + (the "License"); you may not use this file except in compliance with + the License. You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + + Unless required by applicable law or agreed to in writing, software + distributed under the License is distributed on an "AS IS" BASIS, + WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + See the License for the specific language governing permissions and + limitations under the License. +==================================================================== */ +package org.apache.poi.stress; + +import java.io.File; +import java.io.IOException; +import java.util.HashMap; +import java.util.Iterator; +import java.util.LinkedHashMap; +import java.util.List; +import java.util.Map; +import java.util.function.BiConsumer; +import java.util.stream.Collectors; +import java.util.stream.Stream; +import java.util.stream.StreamSupport; + +import org.apache.commons.collections4.MultiValuedMap; +import org.apache.commons.collections4.multimap.ArrayListValuedHashMap; +import org.apache.poi.ss.usermodel.Cell; +import org.apache.poi.ss.usermodel.CellType; +import org.apache.poi.ss.usermodel.Row; +import org.apache.poi.ss.usermodel.Sheet; +import org.apache.poi.ss.usermodel.Workbook; +import org.apache.poi.ss.usermodel.WorkbookFactory; + +public class StressMap { + private final MultiValuedMap exMap = new ArrayListValuedHashMap<>(); + private final Map handlerMap = new LinkedHashMap<>(); + + + public void load(File mapFile) throws IOException { + try (Workbook wb = WorkbookFactory.create(mapFile)) { + readExMap(wb.getSheet("Exceptions")); + readHandlerMap(wb.getSheet("Handlers")); + } + } + + public List getHandler(String file) { + // ... failures/handlers lookup doesn't work on windows otherwise + final String uniFile = file.replace('\\', '/'); + + String firstHandler = handlerMap.entrySet().stream() + .filter(me -> uniFile.endsWith(me.getKey())) + .map(Map.Entry::getValue).findFirst().orElse("NULL"); + + return Stream.of(firstHandler, secondHandler(firstHandler)) + .filter(h -> !"NULL".equals(h)) + .map(FileHandlerKnown::valueOf) + .collect(Collectors.toList()); + } + + public ExcInfo getExcInfo(String file, String testName, FileHandlerKnown handler) { + return exMap.get(file).stream() + .filter(e -> e.isMatch(testName, handler.name())) + .findFirst().orElse(null); + } + + public void readHandlerMap(Sheet sh) { + if (sh == null) { + return; + } + + handlerMap.clear(); + + boolean IGNORE_SCRATCHPAD = Boolean.getBoolean("scratchpad.ignore"); + boolean isFirst = true; + for (Row row : sh) { + if (isFirst) { + isFirst = false; + continue; + } + Cell cell = row.getCell(2); + if (IGNORE_SCRATCHPAD || cell == null || cell.getCellType() != CellType.STRING) { + cell = row.getCell(1); + } + handlerMap.put(row.getCell(0).getStringCellValue(), cell.getStringCellValue()); + } + } + + + public void readExMap(Sheet sh) { + if (sh == null) { + return; + } + + exMap.clear(); + + Iterator iter = sh.iterator(); + List> cols = initCols(iter.next()); + + while (iter.hasNext()) { + ExcInfo info = new ExcInfo(); + for (Cell cell : iter.next()) { + if (cell.getCellType() == CellType.STRING) { + cols.get(cell.getColumnIndex()).accept(info, cell.getStringCellValue()); + } + } + exMap.put(info.getFile(), info); + } + } + + private static List> initCols(Row row) { + Map> m = new HashMap<>(); + m.put("File", ExcInfo::setFile); + m.put("Tests", ExcInfo::setTests); + m.put("Handler", ExcInfo::setHandler); + m.put("Password", ExcInfo::setPassword); + m.put("Exception Class", ExcInfo::setExClazz); + m.put("Exception Message", ExcInfo::setExMessage); + + return StreamSupport + .stream(row.spliterator(), false) + .map(Cell::getStringCellValue) + .map(v -> m.getOrDefault(v, (e,s) -> {})) + .collect(Collectors.toList()); + } + + private static String secondHandler(String handlerStr) { + switch (handlerStr) { + case "XSSF": + case "XWPF": + case "XSLF": + case "XDGF": + return "OPC"; + case "HSSF": + case "HWPF": + case "HSLF": + case "HDGF": + case "HSMF": + case "HBPF": + return "HPSF"; + default: + return "NULL"; + } + } +} diff --git a/src/integrationtest/org/apache/poi/stress/TestAllFiles.java b/src/integrationtest/org/apache/poi/stress/TestAllFiles.java index a065309fc6..8c6e7b98ef 100644 --- a/src/integrationtest/org/apache/poi/stress/TestAllFiles.java +++ b/src/integrationtest/org/apache/poi/stress/TestAllFiles.java @@ -31,26 +31,10 @@ import java.io.FileInputStream; import java.io.IOException; import java.io.InputStream; import java.util.ArrayList; -import java.util.HashMap; -import java.util.Iterator; -import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; -import java.util.function.BiConsumer; -import java.util.function.Supplier; -import java.util.stream.Collectors; import java.util.stream.Stream; -import java.util.stream.StreamSupport; -import org.apache.commons.collections4.MultiValuedMap; -import org.apache.commons.collections4.multimap.ArrayListValuedHashMap; import org.apache.poi.hssf.record.crypto.Biff8EncryptionKey; -import org.apache.poi.ss.usermodel.Cell; -import org.apache.poi.ss.usermodel.CellType; -import org.apache.poi.ss.usermodel.Row; -import org.apache.poi.ss.usermodel.Sheet; -import org.apache.poi.ss.usermodel.Workbook; -import org.apache.poi.ss.usermodel.WorkbookFactory; import org.apache.tools.ant.DirectoryScanner; import org.junit.jupiter.api.function.Executable; import org.junit.jupiter.api.parallel.Execution; @@ -98,12 +82,8 @@ public class TestAllFiles { }; public static Stream allfiles(String testName) throws IOException { - MultiValuedMap exMap; - Map handlerMap; - try (Workbook wb = WorkbookFactory.create(new File(ROOT_DIR, "spreadsheet/stress.xls"))) { - exMap = readExMap(wb.getSheet("Exceptions")); - handlerMap = readHandlerMap(wb.getSheet("Handlers")); - } + StressMap sm = new StressMap(); + sm.load(new File(ROOT_DIR, "spreadsheet/stress.xls")); DirectoryScanner scanner = new DirectoryScanner(); scanner.setBasedir(ROOT_DIR); @@ -113,29 +93,16 @@ public class TestAllFiles { final List result = new ArrayList<>(100); for (String file : scanner.getIncludedFiles()) { - // ... failures/handlers lookup doesn't work on windows otherwise - final String uniFile = file.replace('\\', '/'); - - String firstHandler = handlerMap.entrySet().stream() - .filter(me -> uniFile.endsWith(me.getKey())) - .map(Map.Entry::getValue).findFirst().orElse("NULL"); - - final String[] handlerStr = { firstHandler, secondHandler(firstHandler) }; - for (String hs : handlerStr) { - if ("NULL".equals(hs)) continue; - ExcInfo info1 = exMap.get(file).stream() - .filter(e -> - (e.tests == null || e.tests.contains(testName) || "IGNORE".equals(e.tests)) && - (e.handler == null || e.handler.contains(hs)) - ).findFirst().orElse(null); - - if (info1 == null || !"IGNORE".equals(info1.tests)) { + // if (!file.contains("44958.xls")) continue; + for (FileHandlerKnown handler : sm.getHandler(file)) { + ExcInfo info1 = sm.getExcInfo(file, testName, handler); + if (info1 == null || info1.isValid(testName, handler.name())) { result.add(Arguments.of( file, - hs, - (info1 != null) ? info1.password : null, - (info1 != null) ? info1.exClazz : null, - (info1 != null) ? info1.exMessage : null + handler, + (info1 != null) ? info1.getPassword() : null, + (info1 != null) ? info1.getExClazz() : null, + (info1 != null) ? info1.getExMessage() : null )); } } @@ -150,12 +117,12 @@ public class TestAllFiles { @ParameterizedTest(name = "#{index} {0} {1}") @MethodSource("extractFiles") - void handleExtracting(String file, String handler, String password, Class exClass, String exMessage) throws IOException { + void handleExtracting(String file, FileHandlerKnown handler, String password, Class exClass, String exMessage) throws IOException { System.out.println("Running extractFiles on "+file); - FileHandler fileHandler = Handler.valueOf(handler).fileHandler.get(); + FileHandler fileHandler = handler.fileHandler.get(); assertNotNull(fileHandler, "Did not find a handler for file " + file); Executable exec = () -> fileHandler.handleExtracting(new File(ROOT_DIR, file)); - verify(exec, exClass, exMessage, password); + verify(file, exec, exClass, exMessage, password); } @@ -165,13 +132,13 @@ public class TestAllFiles { @ParameterizedTest(name = "#{index} {0} {1}") @MethodSource("handleFiles") - void handleFile(String file, String handler, String password, Class exClass, String exMessage) throws IOException { + void handleFile(String file, FileHandlerKnown handler, String password, Class exClass, String exMessage) throws IOException { System.out.println("Running handleFiles on "+file); - FileHandler fileHandler = Handler.valueOf(handler).fileHandler.get(); + FileHandler fileHandler = handler.fileHandler.get(); assertNotNull(fileHandler, "Did not find a handler for file " + file); try (InputStream stream = new BufferedInputStream(new FileInputStream(new File(ROOT_DIR, file)), 64 * 1024)) { Executable exec = () -> fileHandler.handleFile(stream, file); - verify(exec, exClass, exMessage, password); + verify(file, exec, exClass, exMessage, password); } } @@ -181,170 +148,43 @@ public class TestAllFiles { @ParameterizedTest(name = "#{index} {0} {1}") @MethodSource("handleAdditionals") - void handleAdditional(String file, String handler, String password, Class exClass, String exMessage) { + void handleAdditional(String file, FileHandlerKnown handler, String password, Class exClass, String exMessage) { System.out.println("Running additionals on "+file); - FileHandler fileHandler = Handler.valueOf(handler).fileHandler.get(); + FileHandler fileHandler = handler.fileHandler.get(); assertNotNull(fileHandler, "Did not find a handler for file " + file); Executable exec = () -> fileHandler.handleAdditional(new File(ROOT_DIR, file)); - verify(exec, exClass, exMessage, password); + verify(file, exec, exClass, exMessage, password); } @SuppressWarnings("unchecked") - private static void verify(Executable exec, Class exClass, String exMessage, String password) { + private static void verify(String file, Executable exec, Class exClass, String exMessage, String password) { + final String errPrefix = file + " - failed. "; // this also removes the password for non encrypted files Biff8EncryptionKey.setCurrentUserPassword(password); if (exClass != null && AssertionFailedError.class.isAssignableFrom(exClass)) { try { exec.execute(); - fail("expected failed assertion"); + fail(errPrefix + "Expected failed assertion"); } catch (AssertionFailedError e) { - assertEquals(exMessage, e.getMessage()); + assertEquals(exMessage, e.getMessage(), errPrefix); } catch (Throwable e) { - fail("unexpected exception", e); + fail(errPrefix + "Unexpected exception", e); } } else if (exClass != null) { Exception e = assertThrows((Class)exClass, exec); String actMsg = e.getMessage(); - if ((NullPointerException.class.isAssignableFrom(exClass) && jreVersion < 16) || exMessage == null) { - assertNull(actMsg); + if (NullPointerException.class.isAssignableFrom(exClass)) { + // with Java 16+ NullPointerExceptions may contain a message ... but apparently not always ?! + assertTrue(jreVersion >= 16 || actMsg == null, errPrefix); + if (actMsg != null) { + assertTrue(actMsg.startsWith(exMessage), errPrefix + "Message: "+actMsg+" - didn't start with "+exMessage); + } } else { - assertNotNull(actMsg); - assertTrue(actMsg.startsWith(exMessage), "Message: "+actMsg+" - didn't start with "+exMessage); + assertNotNull(actMsg, errPrefix); + assertTrue(actMsg.startsWith(exMessage), errPrefix + "Message: "+actMsg+" - didn't start with "+exMessage); } } else { - assertDoesNotThrow(exec); - } - } - - - private static String secondHandler(String handlerStr) { - switch (handlerStr) { - case "XSSF": - case "XWPF": - case "XSLF": - case "XDGF": - return "OPC"; - case "HSSF": - case "HWPF": - case "HSLF": - case "HDGF": - case "HSMF": - case "HBPF": - return "HPSF"; - default: - return "NULL"; - } - } - - private static Map readHandlerMap(Sheet sh) { - Map handlerMap = new LinkedHashMap<>(); - boolean IGNORE_SCRATCHPAD = Boolean.getBoolean("scratchpad.ignore"); - boolean isFirst = true; - for (Row row : sh) { - if (isFirst) { - isFirst = false; - continue; - } - Cell cell = row.getCell(2); - if (IGNORE_SCRATCHPAD || cell == null || cell.getCellType() != CellType.STRING) { - cell = row.getCell(1); - } - handlerMap.put(row.getCell(0).getStringCellValue(), cell.getStringCellValue()); - } - return handlerMap; - } - - - private static MultiValuedMap readExMap(Sheet sh) { - MultiValuedMap exMap = new ArrayListValuedHashMap<>(); - - Iterator iter = sh.iterator(); - List> cols = initCols(iter.next()); - - while (iter.hasNext()) { - ExcInfo info = new ExcInfo(); - for (Cell cell : iter.next()) { - if (cell.getCellType() == CellType.STRING) { - cols.get(cell.getColumnIndex()).accept(info, cell.getStringCellValue()); - } - } - exMap.put(info.file, info); - } - return exMap; - } - - - private static List> initCols(Row row) { - Map> m = new HashMap<>(); - m.put("File", (e,s) -> e.file = s); - m.put("Tests", (e,s) -> e.tests = s); - m.put("Handler", (e,s) -> e.handler = s); - m.put("Password", (e,s) -> e.password = s); - m.put("Exception Class", (e,s) -> { - try { - e.exClazz = (Class) Class.forName(s); - } catch (ClassNotFoundException ex) { - fail(ex); - } - }); - m.put("Exception Message", (e,s) -> e.exMessage = s); - - return StreamSupport - .stream(row.spliterator(), false) - .map(Cell::getStringCellValue) - .map(v -> m.getOrDefault(v, (e,s) -> {})) - .collect(Collectors.toList()); - } - - private static class ExcInfo { - String file; - String tests; - String handler; - String password; - Class exClazz; - String exMessage; - - - } - - @SuppressWarnings("unused") - private enum Handler { - HDGF(HDGFFileHandler::new), - HMEF(HMEFFileHandler::new), - HPBF(HPBFFileHandler::new), - HPSF(HPSFFileHandler::new), - HSLF(HSLFFileHandler::new), - HSMF(HSMFFileHandler::new), - HSSF(HSSFFileHandler::new), - HWPF(HWPFFileHandler::new), - OPC(OPCFileHandler::new), - POIFS(POIFSFileHandler::new), - XDGF(XDGFFileHandler::new), - XSLF(XSLFFileHandler::new), - XSSFB(XSSFBFileHandler::new), - XSSF(XSSFFileHandler::new), - XWPF(XWPFFileHandler::new), - OWPF(OWPFFileHandler::new), - NULL(NullFileHandler::new) - ; - - final Supplier fileHandler; - Handler(Supplier fileHandler) { - this.fileHandler = fileHandler; - } - } - - public static class NullFileHandler implements FileHandler { - @Override - public void handleFile(InputStream stream, String path) { - } - - @Override - public void handleExtracting(File file) { - } - - @Override - public void handleAdditional(File file) { + assertDoesNotThrow(exec, errPrefix); } } } diff --git a/test-data/spreadsheet/stress.xls b/test-data/spreadsheet/stress.xls index 065ad65564..a87e3ff52e 100644 Binary files a/test-data/spreadsheet/stress.xls and b/test-data/spreadsheet/stress.xls differ