Skip to content

Commit

Permalink
Reduce test flakiness (#202)
Browse files Browse the repository at this point in the history
* Make tests less flaky. The biggest change for this is having datasets in a test EDDTestDataset.get{id}() request wait for file copies/data caching to finish instead of throwing an error due to no files (or using whatever subset of files were copied). There's also a fix for metadata in EDDTable where the max value was incorrect in some circumstances when loading a dataset without a cache.

In some error cases table writers were not getting closed. This possible leak of file handles is fixed.

* Fix zarr files

* Format code, set SAX parser to true for tests.

* Have tests handle differences on different machines better.

* fix check for testPmelTaoAirt

* Deflake intetgration tests on linux (and turn off the saveopendap test due to external server)

* Clean up changes relating to testing XInclude (XInclude testing/example will come later).

---------

Co-authored-by: Chris John <Chris.P.John@gmail.com>
  • Loading branch information
ChrisJohnNOAA and ChrisPJohn authored Sep 24, 2024
1 parent ce7a378 commit 5a0d0c8
Show file tree
Hide file tree
Showing 55 changed files with 875 additions and 1,334 deletions.
11 changes: 6 additions & 5 deletions WEB-INF/classes/com/cohort/array/StringArray.java
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,6 @@
import com.cohort.util.StringHolder;
import com.cohort.util.StringHolderComparator;
import com.cohort.util.StringHolderComparatorIgnoreCase;
import ucar.ma2.StructureData;

import java.io.BufferedReader;
import java.io.BufferedWriter;
import java.io.DataInputStream;
Expand All @@ -34,6 +32,7 @@
import java.util.Iterator;
import java.util.Set;
import java.util.regex.Pattern;
import ucar.ma2.StructureData;

/**
* StringArray is a thin shell over a String[] with methods like ArrayList's methods; it extends
Expand Down Expand Up @@ -320,7 +319,8 @@ public static StringArray fromFile(final String fileName, final String charset)
Math2.ensureMemoryAvailable(
File2.length(fileName), "StringArray.fromFile"); // canonical may lessen memory requirement
final StringArray sa = new StringArray();
try (final BufferedReader bufferedReader = File2.getDecompressedBufferedFileReader(fileName, charset);) {
try (final BufferedReader bufferedReader =
File2.getDecompressedBufferedFileReader(fileName, charset); ) {
String s = bufferedReader.readLine();
while (s != null) { // null = end-of-file
sa.addNotCanonical(s);
Expand All @@ -339,10 +339,11 @@ public static StringArray fromFile(final String fileName, final String charset)
* usually all different).
* @throws Exception if trouble (e.g., file not found)
*/
public static StringArray fromFile(final URL resourceFile, final String charset) throws Exception {
public static StringArray fromFile(final URL resourceFile, final String charset)
throws Exception {
final StringArray sa = new StringArray();
try (InputStreamReader reader = new InputStreamReader(resourceFile.openStream(), charset);
BufferedReader bufferedReader = new BufferedReader(reader)){
BufferedReader bufferedReader = new BufferedReader(reader)) {
String s = bufferedReader.readLine();
while (s != null) { // null = end-of-file
sa.addNotCanonical(s);
Expand Down
54 changes: 35 additions & 19 deletions WEB-INF/classes/com/cohort/util/File2.java
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,6 @@
import java.util.zip.GZIPInputStream;
import java.util.zip.ZipEntry;
import java.util.zip.ZipInputStream;

import com.google.common.io.Resources;
import gov.noaa.pfel.erddap.util.EDStatic;
import org.apache.commons.compress.archivers.ArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveEntry;
import org.apache.commons.compress.archivers.tar.TarArchiveInputStream;
Expand Down Expand Up @@ -309,9 +306,9 @@ public static String getClassPath() {

// on windows, remove the troublesome leading "/"
if (String2.OSIsWindows
&& classPath.length() > 2
&& classPath.charAt(0) == '/'
&& classPath.charAt(2) == ':') classPath = classPath.substring(1);
&& classPath.length() > 2
&& classPath.charAt(0) == '/'
&& classPath.charAt(2) == ':') classPath = classPath.substring(1);

// classPath is a URL! so spaces are encoded as %20 on Windows!
// UTF-8: see https://en.wikipedia.org/wiki/Percent-encoding#Current_standard
Expand Down Expand Up @@ -339,7 +336,7 @@ private static String lookupWebInfParentDirectory() {
int po = classPath.indexOf("/WEB-INF/");
if (po < 0)
throw new RuntimeException(
String2.ERROR + ": '/WEB-INF/' not found in classPath=" + classPath);
String2.ERROR + ": '/WEB-INF/' not found in classPath=" + classPath);
classPath = classPath.substring(0, po + 1);
Path path;
if (classPath.startsWith("file:/")) {
Expand All @@ -362,8 +359,8 @@ public static void setWebInfParentDirectory(String webInfParentDir) {
}

/**
* Access a classpath resource via a filesystem path.
* NOTE: this will not work unless resource is exploded.
* Access a classpath resource via a filesystem path. NOTE: this will not work unless resource is
* exploded.
*
* @param resourcePath Classpath of resource.
* @return Filesystem path.
Expand Down Expand Up @@ -795,14 +792,33 @@ public static void rename(String fullOldName, String fullNewName) throws Runtime
if (String2.OSIsWindows)
Math2.sleep(Math2.shortSleep); // if Windows: encourage successful file deletion
}

// rename
if (oldFile.renameTo(newFile)) return;

// failed? give it a second try. This fixed a problem in a test on Windows.
// The problem might be that something needs to be gc'd.
Math2.gcAndWait("File2.rename (before retry)");
if (oldFile.renameTo(newFile)) return;

// This is a bad idea, but its better than datasets failing to load.
try {
Thread.sleep(5000);
} catch (InterruptedException e) {
String2.log("Was sleeping to allow file handles time to free, but got interrupted.");
}
Math2.gcAndWait("File2.rename (before retry)");

if (oldFile.renameTo(newFile)) return;

if (!oldFile.canWrite()) {
throw new RuntimeException(
"Unable to rename\n"
+ fullOldName
+ " to\n"
+ fullNewName
+ "\nbecause the source file is not writable.");
}

throw new RuntimeException("Unable to rename\n" + fullOldName + " to\n" + fullNewName);
}

Expand Down Expand Up @@ -1219,7 +1235,7 @@ public static BufferedInputStream getBufferedInputStream(
* @throws Exception if trouble
*/
public static InputStream getDecompressedBufferedInputStream(String fullFileName, InputStream is)
throws Exception {
throws Exception {
String ext = getExtension(fullFileName); // if e.g., .tar.gz, this returns .gz

// !!!!! IF CHANGE SUPPORTED COMPRESSION TYPES, CHANGE isDecompressible ABOVE !!!
Expand All @@ -1238,8 +1254,8 @@ public static InputStream getDecompressedBufferedInputStream(String fullFileName
if (ext.indexOf('z') < 0) return is;

if (ext.equals(".tgz")
|| fullFileName.endsWith(".tar.gz")
|| fullFileName.endsWith(".tar.gzip")) {
|| fullFileName.endsWith(".tar.gz")
|| fullFileName.endsWith(".tar.gzip")) {
// modified from
// https://stackoverflow.com/questions/7128171/how-to-compress-decompress-tar-gz-files-in-java
GzipCompressorInputStream gzipIn = null;
Expand All @@ -1251,7 +1267,7 @@ public static InputStream getDecompressedBufferedInputStream(String fullFileName
while (entry != null && entry.isDirectory()) entry = tarIn.getNextTarEntry();
if (entry == null)
throw new IOException(
String2.ERROR + " while reading " + fullFileName + ": no file found in archive.");
String2.ERROR + " while reading " + fullFileName + ": no file found in archive.");
is = tarIn;
} catch (Exception e) {
if (tarIn != null) tarIn.close();
Expand All @@ -1276,7 +1292,7 @@ public static InputStream getDecompressedBufferedInputStream(String fullFileName
while (entry != null && entry.isDirectory()) entry = zis.getNextEntry();
if (entry == null)
throw new IOException(
String2.ERROR + " while reading " + fullFileName + ": no file found in archive.");
String2.ERROR + " while reading " + fullFileName + ": no file found in archive.");
is = zis;
} catch (Exception e) {
if (zis != null) zis.close();
Expand Down Expand Up @@ -1323,8 +1339,7 @@ public static InputStream getDecompressedBufferedInputStream(String fullFileName
* @return a decompressed, buffered InputStream from a file.
* @throws Exception if trouble
*/
public static InputStream getDecompressedBufferedInputStream(URL resourceFile)
throws Exception {
public static InputStream getDecompressedBufferedInputStream(URL resourceFile) throws Exception {
return getDecompressedBufferedInputStream(resourceFile.getFile(), resourceFile.openStream());
}

Expand Down Expand Up @@ -1762,6 +1777,7 @@ public static ArrayList<String> readLinesFromFile(String fileName, String charse
if (bufferedReader != null) bufferedReader.close();
}
}

/**
* This is like the other readFromFile, but returns ArrayList of Strings and throws Exception is
* trouble. The strings in the ArrayList are not canonical! So this is useful for reading,
Expand All @@ -1777,8 +1793,8 @@ public static ArrayList<String> readLinesFromFile(String fileName, String charse
* @return ArrayList with the lines from the file
* @throws Exception if trouble
*/
public static ArrayList<String> readLinesFromFile(URL resourceFile, String charset, int maxAttempt)
throws Exception {
public static ArrayList<String> readLinesFromFile(
URL resourceFile, String charset, int maxAttempt) throws Exception {

long time = System.currentTimeMillis();
BufferedReader bufferedReader = null;
Expand Down
10 changes: 3 additions & 7 deletions WEB-INF/classes/com/cohort/util/Units2.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,12 @@
import com.cohort.array.PAType;
import com.cohort.array.PrimitiveArray;
import com.cohort.array.StringArray;

import com.google.common.io.Resources;
import java.net.URL;
import java.nio.charset.Charset;
import java.util.HashMap;
import java.util.List;
import java.util.Map;

import com.google.common.io.Resources;
import ucar.units.Unit;
import ucar.units.UnitFormat;

Expand Down Expand Up @@ -55,12 +53,10 @@ public class Units2 {
// these don't need to be thread-safe because they are read-only after creation
private static Map<String, String> udHashMap =
getHashMapStringString(
Resources.getResource("com/cohort/util/UdunitsToUcum.properties"),
File2.UTF_8);
Resources.getResource("com/cohort/util/UdunitsToUcum.properties"), File2.UTF_8);
protected static Map<String, String> ucHashMap =
getHashMapStringString(
Resources.getResource("com/cohort/util/UcumToUdunits.properties"),
File2.UTF_8);
Resources.getResource("com/cohort/util/UcumToUdunits.properties"), File2.UTF_8);

// these special cases are usually populated by EDStatic static constructor, but don't have to be
public static Map<String, String> standardizeUdunitsHM = new HashMap();
Expand Down
23 changes: 8 additions & 15 deletions WEB-INF/classes/com/cohort/util/XML.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,6 @@
*/
package com.cohort.util;

import org.w3c.dom.Document;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import org.xml.sax.InputSource;

import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.xpath.XPath;
import java.io.BufferedReader;
import java.io.InputStream;
import java.io.InputStreamReader;
Expand All @@ -19,6 +12,12 @@
import java.net.URL;
import java.nio.charset.StandardCharsets;
import java.util.HashMap;
import javax.xml.parsers.DocumentBuilderFactory;
import javax.xml.xpath.XPath;
import org.w3c.dom.Document;
import org.w3c.dom.Node;
import org.w3c.dom.NodeList;
import org.xml.sax.InputSource;

/** This has some static methods to facilitate reading and writing an XML file. */
public class XML {
Expand Down Expand Up @@ -795,10 +794,7 @@ public static void removeComments(StringBuilder document) {
*/
public static Document parseXml(String fileName, boolean validating) throws Exception {
BufferedReader reader = File2.getDecompressedBufferedFileReader(fileName, File2.UTF_8);
return parseXml(
new InputSource(reader),
validating
);
return parseXml(new InputSource(reader), validating);
}

/**
Expand All @@ -813,10 +809,7 @@ public static Document parseXml(String fileName, boolean validating) throws Exce
public static Document parseXml(URL resourceFile, boolean validating) throws Exception {
InputStream decompressedStream = File2.getDecompressedBufferedInputStream(resourceFile);
InputStreamReader reader = new InputStreamReader(decompressedStream, StandardCharsets.UTF_8);
return parseXml(
new InputSource(new BufferedReader(reader)),
validating
);
return parseXml(new InputSource(new BufferedReader(reader)), validating);
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,6 @@
import gov.noaa.pfel.coastwatch.util.RegexFilenameFilter;
import gov.noaa.pfel.coastwatch.util.SSR;

import java.nio.file.Paths;

/**
* This class is designed to be a stand-alone program to validate that the DataSet.properties file
* contains valid information for all datasets listed by validDataSets in DataSet.properties. Don't
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
import com.google.common.io.Resources;
import gov.noaa.pfel.coastwatch.TimePeriods;
import gov.noaa.pfel.coastwatch.util.SSR;

import java.net.URL;
import java.util.GregorianCalendar;

Expand Down Expand Up @@ -57,7 +56,7 @@ public class FileNameUtility {
public double regionMinX, regionMaxX, regionMinY, regionMaxY;

public static URL STANDARD_REGIONS_FILE_NAME =
Resources.getResource("gov/noaa/pfel/coastwatch/griddata/regions");
Resources.getResource("gov/noaa/pfel/coastwatch/griddata/regions");

public static String getAcknowledgement() {
return "NOAA NESDIS COASTWATCH, NOAA SWFSC ERD";
Expand Down
3 changes: 1 addition & 2 deletions WEB-INF/classes/gov/noaa/pfel/coastwatch/hdf/SdsReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import com.cohort.util.String2;
import com.cohort.util.Test;
import com.google.common.io.Resources;

import java.io.DataInputStream;
import java.util.ArrayList;

Expand Down Expand Up @@ -211,7 +210,7 @@ public static void main(String args[]) throws Exception {

// mini.hdf was made by gov.noaa.pfel.coastwatch/Grid with the HDF libraries
String resourcePath = Resources.getResource("gov/noaa/pfel/coastwatch/hdf/").toString();
String dir = File2.accessResourceFile(resourcePath);
String dir = File2.accessResourceFile(resourcePath);
// Test.ensureEqual(
// File2.writeToFileUtf8(dir + "mini.hdf.hexDump", File2.hexDump(dir + "mini.hdf", 7262)),
// "", "Grid.miniTestSaveAsHDF error message");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import com.cohort.util.String2;
import com.cohort.util.Test;
import com.google.common.io.Resources;

import java.io.BufferedOutputStream;
import java.io.DataOutputStream;
import java.io.FileOutputStream;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import com.google.common.io.Resources;
import gov.noaa.pmel.sgt.ColorMap;
import gov.noaa.pmel.util.Range2D;

import java.awt.Color;
import java.net.URL;
import java.nio.charset.StandardCharsets;
Expand Down Expand Up @@ -156,9 +155,11 @@ public CompoundColorMap(URL cptFileName) throws Exception {
}

/**
* This constructs a CompoundColorMap based on a .cpt file. It is used by some of the constructors.
* This constructs a CompoundColorMap based on a .cpt file. It is used by some of the
* constructors.
*/
protected static void populate(CompoundColorMap ccm, String cptFileName, List<String> lines) throws Exception {
protected static void populate(CompoundColorMap ccm, String cptFileName, List<String> lines)
throws Exception {
// set up a colorMap based on info in the .cpt file
// set up temporary PrimitiveArrays
DoubleArray rangeLowAr = new DoubleArray(); // stores the low ends of a piece
Expand Down Expand Up @@ -465,7 +466,8 @@ public CompoundColorMap(
// public static String makeCPT(String baseDir, String palette, String scale, double minData,
// double maxData, int nSections, boolean continuous, String resultDir) throws Exception {
if (reallyVerbose) String2.log("nPieces=" + nPieces);
String cptFileName = makeCPT(baseDir, palette, "Linear", 0, nPieces, nPieces, continuous, resultDir);
String cptFileName =
makeCPT(baseDir, palette, "Linear", 0, nPieces, nPieces, continuous, resultDir);
List<String> lines = File2.readLinesFromFile(cptFileName, File2.ISO_8859_1, 3);
populate(this, cptFileName, lines);

Expand Down
1 change: 0 additions & 1 deletion WEB-INF/classes/gov/noaa/pfel/coastwatch/sgt/GSHHS.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
import com.cohort.util.LRUCache;
import com.cohort.util.String2;
import gov.noaa.pfel.erddap.util.EDStatic;

import java.awt.geom.GeneralPath;
import java.io.*;
import java.util.Collections;
Expand Down
10 changes: 5 additions & 5 deletions WEB-INF/classes/gov/noaa/pfel/coastwatch/sgt/SgtMap.java
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ public class SgtMap {
* (https://www.ngdc.noaa.gov/mgg/shorelines/gshhs.html). landMaskDir should have slash at end.
*/
public static String fullRefDirectory =
EDStatic.getWebInfParentDirectory()
EDStatic.getWebInfParentDirectory()
+ // with / separator and / at the end
"WEB-INF/ref/";

Expand Down Expand Up @@ -165,7 +165,7 @@ public class SgtMap {

public static String bathymetryCptTrue = "OceanTrue.cpt";
public static URL bathymetryCptFullName =
Resources.getResource("gov/noaa/pfel/coastwatch/sgt/" + bathymetryCpt);
Resources.getResource("gov/noaa/pfel/coastwatch/sgt/" + bathymetryCpt);
public static URL bathymetryCptTrueFullName =
Resources.getResource("gov/noaa/pfel/coastwatch/sgt/" + bathymetryCptTrue);

Expand Down Expand Up @@ -422,8 +422,8 @@ public static ArrayList makeMap(
* @param contourAltScaleFactor is a scale factor to be applied to the data (use "1" if none)
* @param contourAltOffset is a scale factor to be added to the data (use "0" if none)
* @param contourDrawLinesAt is a single value or a comma-separated list of values at which
* contour lines should be drawn
* param contourPaletteFileName is the complete name of the palette file to be used
* contour lines should be drawn param contourPaletteFileName is the complete name of the
* palette file to be used
* @param contourColor is an int with the rgb color value for the contour lines
* @param contourBoldTitle
* @param contourUnits
Expand Down Expand Up @@ -2759,7 +2759,7 @@ public static int[] makeRegionsMap(
Grid bathymetryGrid =
createTopographyGrid(
fullPrivateDirectory, minX, maxX, minY, maxY, graphWidth, graphHeight);
URL resourceFile = Resources.getResource("gov/noaa/pfel/coastwatch/sgt/"+ bathymetryCpt);
URL resourceFile = Resources.getResource("gov/noaa/pfel/coastwatch/sgt/" + bathymetryCpt);
CompoundColorMap oceanColorMap = new CompoundColorMap(resourceFile);
graph = new CartesianGraph("", xt, yt);
layer = new Layer("bathymetryColors", layerDimension2D);
Expand Down
Loading

0 comments on commit 5a0d0c8

Please sign in to comment.