Index: trunk/test/unit/org/openstreetmap/josm/gui/preferences/imagery/ImageryPreferenceTestIT.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/gui/preferences/imagery/ImageryPreferenceTestIT.java (revision 15085) +++ trunk/test/unit/org/openstreetmap/josm/gui/preferences/imagery/ImageryPreferenceTestIT.java (revision 15095) @@ -28,8 +28,6 @@ import org.junit.ClassRule; import org.junit.Test; -import org.junit.runner.Description; import org.junit.runner.RunWith; import org.junit.runners.Parameterized.Parameters; -import org.junit.runners.model.Statement; import org.openstreetmap.gui.jmapviewer.Coordinate; import org.openstreetmap.gui.jmapviewer.TileXY; @@ -85,18 +83,5 @@ @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") public static JOSMTestRules test = new JOSMTestRules().https().i18n().preferences().projection().projectionNadGrids() - .timeout((int) TimeUnit.MINUTES.toMillis(40)); - - static { - try { - test.apply(new Statement() { - @Override - public void evaluate() throws Throwable { - // Do nothing. Hack needed because @Parameters are computed before anything else - } - }, Description.createSuiteDescription(ImageryPreferenceTestIT.class)).evaluate(); - } catch (Throwable e) { - Logging.error(e); - } - } + .timeout((int) TimeUnit.MINUTES.toMillis(40)).parameters(); /** Entry to test */ Index: trunk/test/unit/org/openstreetmap/josm/gui/preferences/map/MapPaintPreferenceTestIT.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/gui/preferences/map/MapPaintPreferenceTestIT.java (revision 15085) +++ trunk/test/unit/org/openstreetmap/josm/gui/preferences/map/MapPaintPreferenceTestIT.java (revision 15095) @@ -2,13 +2,18 @@ package org.openstreetmap.josm.gui.preferences.map; -import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeTrue; -import java.util.Collection; -import java.util.HashMap; -import java.util.Map; +import java.io.IOException; +import java.util.ArrayList; +import java.util.List; +import java.util.stream.Collectors; -import org.junit.Rule; +import org.junit.BeforeClass; +import org.junit.ClassRule; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized.Parameters; +import org.openstreetmap.josm.TestUtils; import org.openstreetmap.josm.data.preferences.sources.ExtendedSourceEntry; import org.openstreetmap.josm.gui.mappaint.MapPaintStyles; @@ -21,4 +26,5 @@ import org.openstreetmap.josm.gui.mappaint.mapcss.MapCSSStyleSource; import org.openstreetmap.josm.testutils.JOSMTestRules; +import org.openstreetmap.josm.testutils.ParallelParameterized; import org.openstreetmap.josm.tools.ImageProvider; @@ -28,4 +34,5 @@ * Integration tests of {@link MapPaintPreference} class. */ +@RunWith(ParallelParameterized.class) public class MapPaintPreferenceTestIT { @@ -33,57 +40,83 @@ * Setup rule */ - @Rule + @ClassRule @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") - public JOSMTestRules test = new JOSMTestRules().https().timeout(15000*60); + public static JOSMTestRules test = new JOSMTestRules().https().timeout(15000*60).parameters(); + + /** Entry to test */ + private final ExtendedSourceEntry source; + private final List ignoredErrors = new ArrayList<>(); + private static final List errorsToIgnore = new ArrayList<>(); /** - * Test that available map paint styles are valid. + * Setup test + * @throws IOException in case of I/O error + */ + @BeforeClass + public static void beforeClass() throws IOException { + errorsToIgnore.addAll(TestUtils.getIgnoredErrorMessages(MapPaintPreferenceTestIT.class)); + } + + /** + * Returns list of map paint styles to test. + * @return list of map paint styles to test + * @throws Exception if an error occurs + */ + @Parameters(name = "{0} - {1}") + public static List data() throws Exception { + ImageProvider.clearCache(); + return new MapPaintPreference.MapPaintSourceEditor().loadAndGetAvailableSources().stream() + .map(x -> new Object[] {x.getDisplayName(), x.url, x}).collect(Collectors.toList()); + } + + /** + * Constructs a new {@code MapPaintPreferenceTestIT} + * @param displayName displayed name + * @param url URL + * @param source source entry to test + */ + public MapPaintPreferenceTestIT(String displayName, String url, ExtendedSourceEntry source) { + this.source = source; + } + + /** + * Test that map paint style is valid. * @throws Exception in case of error */ @Test - public void testValidityOfAvailableStyles() throws Exception { - ImageProvider.clearCache(); - Collection sources = new MapPaintPreference.MapPaintSourceEditor() - .loadAndGetAvailableSources(); - // Drop everything from yopaseopor, pasharm and www.freietonne.de, too many errors - sources.removeIf(x -> x.url.contains("yopaseopor/") || x.url.contains("/pasharm") || x.url.contains("www.freietonne.de")); - assertFalse(sources.isEmpty()); - Map> allErrors = new HashMap<>(); - Map> allWarnings = new HashMap<>(); - for (ExtendedSourceEntry source : sources) { - // Do not validate XML styles - if (!"xml".equalsIgnoreCase(source.styleType)) { - System.out.println(source.url); - StyleSource style = MapPaintStyles.addStyle(source); - if (style instanceof MapCSSStyleSource) { - // Force loading of all icons to detect missing ones - for (MapCSSRule rule : ((MapCSSStyleSource) style).rules) { - for (Instruction instruction : rule.declaration.instructions) { - if (instruction instanceof AssignmentInstruction) { - AssignmentInstruction ai = (AssignmentInstruction) instruction; - if (StyleKeys.ICON_IMAGE.equals(ai.key) - || StyleKeys.FILL_IMAGE.equals(ai.key) - || StyleKeys.REPEAT_IMAGE.equals(ai.key)) { - if (ai.val instanceof String) { - MapPaintStyles.getIconProvider(new IconReference((String) ai.val, style), true); - } - } + public void testStyleValidity() throws Exception { + StyleSource style = MapPaintStyles.addStyle(source); + if (style instanceof MapCSSStyleSource) { + // Force loading of all icons to detect missing ones + for (MapCSSRule rule : ((MapCSSStyleSource) style).rules) { + for (Instruction instruction : rule.declaration.instructions) { + if (instruction instanceof AssignmentInstruction) { + AssignmentInstruction ai = (AssignmentInstruction) instruction; + if (StyleKeys.ICON_IMAGE.equals(ai.key) + || StyleKeys.FILL_IMAGE.equals(ai.key) + || StyleKeys.REPEAT_IMAGE.equals(ai.key)) { + if (ai.val instanceof String) { + MapPaintStyles.getIconProvider(new IconReference((String) ai.val, style), true); } } } } - System.out.println(style.isValid() ? " => OK" : " => KO"); - Collection errors = style.getErrors(); - Collection warnings = style.getWarnings(); - if (!errors.isEmpty()) { - allErrors.put(source.url, errors); - } - if (!warnings.isEmpty()) { - allWarnings.put(source.url, warnings); - } } } - ImageProvider.clearCache(); - assertTrue(allErrors.toString()+"\n"+allWarnings.toString(), allErrors.isEmpty() && allWarnings.isEmpty()); + + List errors = new ArrayList<>(style.getErrors()); + errors.stream().map(Throwable::getMessage).filter(MapPaintPreferenceTestIT::isIgnoredSubstring).forEach(ignoredErrors::add); + errors.removeIf(e -> ignoredErrors.contains(e.getMessage())); + + List warnings = new ArrayList<>(style.getWarnings()); + warnings.stream().filter(MapPaintPreferenceTestIT::isIgnoredSubstring).forEach(ignoredErrors::add); + warnings.removeAll(ignoredErrors); + + assertTrue(errors.toString() + '\n' + warnings.toString(), errors.isEmpty() && warnings.isEmpty()); + assumeTrue(ignoredErrors.toString(), ignoredErrors.isEmpty()); + } + + private static boolean isIgnoredSubstring(String substring) { + return errorsToIgnore.parallelStream().anyMatch(x -> substring.contains(x)); } } Index: trunk/test/unit/org/openstreetmap/josm/gui/preferences/map/TaggingPresetPreferenceTestIT.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/gui/preferences/map/TaggingPresetPreferenceTestIT.java (revision 15085) +++ trunk/test/unit/org/openstreetmap/josm/gui/preferences/map/TaggingPresetPreferenceTestIT.java (revision 15095) @@ -4,18 +4,25 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assume.assumeTrue; import java.io.IOException; +import java.util.ArrayList; import java.util.Collection; -import java.util.HashMap; -import java.util.Map; +import java.util.HashSet; +import java.util.List; +import java.util.Locale; import java.util.Objects; import java.util.Set; -import java.util.TreeSet; import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; +import java.util.stream.Collectors; -import org.junit.Rule; +import org.junit.BeforeClass; +import org.junit.ClassRule; import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized.Parameters; +import org.openstreetmap.josm.TestUtils; import org.openstreetmap.josm.data.preferences.sources.ExtendedSourceEntry; import org.openstreetmap.josm.gui.tagging.presets.TaggingPreset; @@ -23,4 +30,5 @@ import org.openstreetmap.josm.spi.preferences.Config; import org.openstreetmap.josm.testutils.JOSMTestRules; +import org.openstreetmap.josm.testutils.ParallelParameterized; import org.openstreetmap.josm.tools.ImageProvider; import org.openstreetmap.josm.tools.Logging; @@ -32,4 +40,5 @@ * Integration tests of {@link TaggingPresetPreference} class. */ +@RunWith(ParallelParameterized.class) public class TaggingPresetPreferenceTestIT { @@ -37,55 +46,84 @@ * Setup rule */ - @Rule + @ClassRule @SuppressFBWarnings(value = "URF_UNREAD_PUBLIC_OR_PROTECTED_FIELD") - public JOSMTestRules test = new JOSMTestRules().https().timeout(10000*60); + public static JOSMTestRules test = new JOSMTestRules().https().timeout(10000*60).parameters(); + + /** Entry to test */ + private final ExtendedSourceEntry source; + private final List ignoredErrors = new ArrayList<>(); + private static final List errorsToIgnore = new ArrayList<>(); /** - * Test that available tagging presets are valid. + * Setup test + * @throws IOException in case of I/O error + */ + @BeforeClass + public static void beforeClass() throws IOException { + errorsToIgnore.addAll(TestUtils.getIgnoredErrorMessages(TaggingPresetPreferenceTestIT.class)); + // Double traditional timeouts to avoid random problems + Config.getPref().putInt("socket.timeout.connect", 30); + Config.getPref().putInt("socket.timeout.read", 60); + // Make sure error messages are in english + Locale.setDefault(Locale.ENGLISH); + } + + /** + * Returns list of tagging presets to test. + * @return list of tagging presets to test + * @throws Exception if an error occurs + */ + @Parameters(name = "{0} - {1}") + public static List data() throws Exception { + ImageProvider.clearCache(); + return new TaggingPresetPreference.TaggingPresetSourceEditor().loadAndGetAvailableSources().stream() + .map(x -> new Object[] {x.getDisplayName(), x.url, x}).collect(Collectors.toList()); + } + + /** + * Constructs a new {@code TaggingPresetPreferenceTestIT} + * @param displayName displayed name + * @param url URL + * @param source source entry to test + */ + public TaggingPresetPreferenceTestIT(String displayName, String url, ExtendedSourceEntry source) { + this.source = source; + } + + /** + * Test that tagging presets are valid. * @throws Exception in case of error */ @Test - public void testValidityOfAvailablePresets() throws Exception { - ImageProvider.clearCache(); - Collection sources = new TaggingPresetPreference.TaggingPresetSourceEditor() - .loadAndGetAvailableSources(); - assertFalse(sources.isEmpty()); - // Double traditional timeouts to avoid random problems - Config.getPref().putInt("socket.timeout.connect", 30); - Config.getPref().putInt("socket.timeout.read", 60); - Map allErrors = new HashMap<>(); - Map> allMessages = new HashMap<>(); - for (ExtendedSourceEntry source : sources) { - System.out.println(source.url); + public void testPresetsValidity() throws Exception { + Set errors = new HashSet<>(); + try { + testPresets(errors, source); + } catch (IOException e) { try { - testPresets(allMessages, source); - } catch (IOException e) { - try { - Logging.warn(e); - // try again in case of temporary network error - testPresets(allMessages, source); - } catch (SAXException | IOException e1) { - e.printStackTrace(); - // ignore frequent network errors with www.freietonne.de causing too much Jenkins failures - if (!source.url.contains("www.freietonne.de")) { - allErrors.put(source.url, e1); - } - System.out.println(" => KO"); - } - } catch (SAXException | IllegalArgumentException e) { - e.printStackTrace(); - if (!source.url.contains("yopaseopor/")) { - // ignore https://raw.githubusercontent.com/yopaseopor/traffic_signs_preset_JOSM cause too much errors - allErrors.put(source.url, e); - } - System.out.println(" => KO"); + Logging.warn(e); + // try again in case of temporary network error + testPresets(errors, source); + } catch (SAXException | IOException e1) { + handleException(e1, errors); } + } catch (SAXException | IllegalArgumentException e) { + handleException(e, errors); } - ImageProvider.clearCache(); - assertTrue(allErrors.toString(), allErrors.isEmpty()); - assertTrue(allMessages.toString(), allMessages.isEmpty()); + assertTrue(errors.toString(), errors.isEmpty()); + assumeTrue(ignoredErrors.toString(), ignoredErrors.isEmpty()); } - private static void testPresets(Map> allMessages, ExtendedSourceEntry source) throws SAXException, IOException { + private void handleException(Exception e, Set errors) { + e.printStackTrace(); + String s = source.url + " => " + e.toString(); + if (isIgnoredSubstring(s)) { + ignoredErrors.add(s); + } else { + errors.add(s); + } + } + + private void testPresets(Set messages, ExtendedSourceEntry source) throws SAXException, IOException { Collection presets = TaggingPresetReader.readAll(source.url, true); assertFalse(presets.isEmpty()); @@ -103,15 +141,18 @@ if (message.contains(TaggingPreset.PRESET_ICON_ERROR_MSG_PREFIX)) { error = true; - // ignore https://github.com/yopaseopor/traffic_signs_preset_JOSM because of far too frequent missing icons errors - // ignore https://github.com/ruosm-presets because of frequent missing icons errors and no response - if (!source.url.contains("yopaseopor/traffic_signs") && !source.url.contains("ruosm-presets/literan-moscow")) { - allMessages.computeIfAbsent(source.url, x -> new TreeSet<>()).add(message); + if (isIgnoredSubstring(message)) { + ignoredErrors.add(message); + } else { + messages.add(message); } } } - System.out.println(error ? " => KO" : " => OK"); if (error) { Logging.clearLastErrorAndWarnings(); } } + + private static boolean isIgnoredSubstring(String substring) { + return errorsToIgnore.parallelStream().anyMatch(x -> substring.contains(x)); + } } Index: trunk/test/unit/org/openstreetmap/josm/testutils/JOSMTestRules.java =================================================================== --- trunk/test/unit/org/openstreetmap/josm/testutils/JOSMTestRules.java (revision 15085) +++ trunk/test/unit/org/openstreetmap/josm/testutils/JOSMTestRules.java (revision 15095) @@ -45,4 +45,5 @@ import org.openstreetmap.josm.gui.mappaint.MapPaintStyles; import org.openstreetmap.josm.gui.oauth.OAuthAuthorizationWizard; +import org.openstreetmap.josm.gui.preferences.imagery.ImageryPreferenceTestIT; import org.openstreetmap.josm.gui.tagging.presets.TaggingPresets; import org.openstreetmap.josm.gui.util.GuiHelper; @@ -366,4 +367,22 @@ } + /** + * Must be called if test run with Junit parameters + * @return this instance, for easy chaining + */ + public JOSMTestRules parameters() { + try { + apply(new Statement() { + @Override + public void evaluate() throws Throwable { + // Do nothing. Hack needed because @Parameters are computed before anything else + } + }, Description.createSuiteDescription(ImageryPreferenceTestIT.class)).evaluate(); + } catch (Throwable e) { + Logging.error(e); + } + return this; + } + private static class MockVersion extends Version { MockVersion(final String propertiesString) {