Skip to content

Commit

Permalink
Fix tests on windows by Peter Hull
Browse files Browse the repository at this point in the history
Force test resource files to be binary

On Windows, git would treat the unencoded files as text and apply CRLF line endings, so their base 64 encodings would not match the reference files.

Work around timing issues on Windows

On Windows the elapsed time was quite unreliable, causing intermittent fails in the performance tests, presumably made worse because PCs / the JRE are faster now then when this code was written.

Also fixed up a few warnings/javadoc issues and replaced a home-grown bubble sort with Arrays.sort.
  • Loading branch information
pedro-w authored and simonsteiner1984 committed Jun 7, 2024
1 parent 7bd4291 commit 964b035
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 29 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/maven.yml
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ jobs:
fail-fast: false
matrix:
jdk: ['8', '11', '17']
os: [ubuntu-latest]
os: [ubuntu-latest, windows-latest]

steps:
- uses: actions/checkout@v4
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,10 @@ public class JUnitRunnerTestCase {

@BeforeClass
public static void beforeClass() throws IOException {
if (System.getProperty("os.name").startsWith("Windows")) {
EXCLUDE = new ArrayList<>(EXCLUDE);
EXCLUDE.add("PerformanceTestSanity");
}
String sm = "grant { permission java.security.AllPermission; };";
File tmp = File.createTempFile("batik", "sm");
FileOutputStream fos = new FileOutputStream(tmp);
Expand Down
59 changes: 31 additions & 28 deletions batik-test/src/main/java/org/apache/batik/test/PerformanceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Licensed to the Apache Software Foundation (ASF) under one or more
*/
package org.apache.batik.test;

import java.util.Arrays;
import java.util.Vector;

/**
Expand Down Expand Up @@ -74,16 +75,20 @@ public void setAllowedScoreDeviation(double allowedScoreDeviation) {
/**
* Force implementations to only implement <code>runOp</code>
* and other performance specific methods.
* @return the results of AbstractTest's run method
*/
@Override
public final TestReport run() {
return super.run();
}

/**
* Force implementations to only implement <code>runOp</code>
* and other performance specific methods.
* @return false always, indicating failure
*/
public final boolean runImplBasic() throws Exception {
@Override
public final boolean runImplBasic() {
// Should never be called for a PerformanceTest
return false;
}
Expand All @@ -95,27 +100,31 @@ public final boolean runImplBasic() throws Exception {
* or not the score is within the allowed deviation of the
* reference score.
*
* @return the test report
* @throws java.lang.Exception if an error occurred
* @see #runRef
* @see #runOp
*/
@Override
public final TestReport runImpl() throws Exception {
int iter = 50;

double refUnit = 0;
long refStart = 0;
long refEnd = 0;
long opEnd = 0;
long opStart = 0;
double opLength = 0;
double refUnit;
long refStart;
long refEnd;
long opEnd;
long opStart;
double opLength;

// Run once to remove class load time from timing.
runRef();
runOp();
// System.gc();

double[] scores = new double[iter];

for (int i=0; i<iter; i++) {
// Count bad scores so it doesn't get stuck forever
int badScores = 0;
for (int i=0; i<iter; ) {
if ( i%2 == 0) {
refStart = System.currentTimeMillis();
runRef();
Expand All @@ -133,17 +142,26 @@ public final TestReport runImpl() throws Exception {
refUnit = refEnd - opEnd;
opLength = opEnd - opStart;
}
// Only accept finite and non-zero scores
if (opLength > 0.0 && refUnit > 0.0) {
scores[i++] = opLength / refUnit;
System.err.print(".");
} else {
System.err.print("!");
if (++badScores > iter) {
// Too many bad scores
throw new IllegalStateException("Unable to obtain reliable timings");

scores[i] = opLength / refUnit;
System.err.println(".");
}
}
// System.err.println(">>>>>>>> scores[" + i + "] = " + scores[i] + " (" + refUnit + " / " + opLength + ")");
System.gc();
}

System.err.println();

// Now, sort the scores
sort(scores);
Arrays.sort(scores);

// Compute the mean score based on the scores, not accounting
// for the lowest and highest scores
Expand Down Expand Up @@ -184,22 +202,6 @@ public final TestReport runImpl() throws Exception {
}
}

protected void sort(double[] a) throws Exception {
for (int i = a.length - 1; i>=0; i--) {
boolean swapped = false;
for (int j = 0; j<i; j++) {
if (a[j] > a[j+1]) {
double d = a[j];
a[j] = a[j+1];
a[j+1] = d;
swapped = true;
}
}
if (!swapped)
return;
}
}

/**
* Runs the reference operation.
* By default, this runs the same BufferedImage drawing
Expand All @@ -220,6 +222,7 @@ protected void runRef() {

/**
* Runs the tested operation
* @throws java.lang.Exception if an error occurred
*/
protected abstract void runOp() throws Exception;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
* -text

0 comments on commit 964b035

Please sign in to comment.