Skip to content

Commit f1ad890

Browse files
committed
test: consolidate fixture download/cache/skip logic into LocalHttpCache
Five copy-pasted variants of "check a /tmp cache, download over HTTP, skip on transient server error or no network" had drifted apart: RustWritesJavaReadsIntegrationTest and PcoFixtureInspectionIntegrationTest were exact duplicates; ParquetImportIntegrationTest's fixture download had no version-keyed cache or 5xx-skip guard; RustJavaReaderComparison IntegrationTest used JDK HttpClient/BodyHandlers with no cache and no network-skip guard at all (a network blip failed the test instead of skipping it); and taxiParquet_importedSize_vsOriginal reimplemented the same thing inline with fully-qualified java.nio.file.Files calls. Extract LocalHttpCache.downloadIfMissing (skip on 5xx, fail on 4xx) and .assumeNetworkAvailable, plus .downloadIfMissingOrSkip for endpoints that skip on ANY failure (CloudFront occasionally 403s CI runners' egress IPs). Migrate all five call sites, standardizing on HttpURLConnection since four of five already used it. TaxiParquetOracleVsJavaIntegrationTest previously only ran if someone had manually placed /tmp/yellow_tripdata_2024-01.parquet — it never downloaded anything, so it silently skipped in every environment without that manual step. It now shares the download-and-cache path with taxiParquet_importedSize_vsOriginal and actually runs.
1 parent e259261 commit f1ad890

6 files changed

Lines changed: 124 additions & 109 deletions
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
package io.github.dfa1.vortex.integration;
2+
3+
import java.net.HttpURLConnection;
4+
import java.net.URI;
5+
import java.nio.file.Files;
6+
import java.nio.file.Path;
7+
import java.nio.file.StandardCopyOption;
8+
9+
import static org.junit.jupiter.api.Assumptions.assumeTrue;
10+
11+
/// Shared fixture-download-and-cache helper for integration tests that pull test data
12+
/// from S3/GitHub over HTTP: check a local cache first, download on miss, and skip
13+
/// (rather than fail) the test on transient server errors or no network.
14+
final class LocalHttpCache {
15+
16+
private LocalHttpCache() {
17+
}
18+
19+
/// Downloads `url` into `tmp` if `cacheDir` doesn't already have a file named `name`,
20+
/// leaving no copy in `cacheDir` (callers that want the download persisted for reuse by
21+
/// later runs must pass a `cacheDir` that already exists and is writable — this method
22+
/// only reads from it).
23+
///
24+
/// A 5xx response skips the test via `assumeTrue` (assumed transient infrastructure
25+
/// noise, not a real signal); a 4xx or other `IOException` fails it (the fixture is
26+
/// genuinely missing or broken).
27+
///
28+
/// @param tmp the test's `@TempDir`, used as the download destination when the
29+
/// fixture isn't already in `cacheDir`
30+
/// @param cacheDir persistent local cache directory (e.g. `/tmp/pco-fixtures/v0.75.0`)
31+
/// @param url the fixture's download URL
32+
/// @param name the fixture's file name, used for both the cache lookup and dest file
33+
/// @return the local path to the fixture: the cached copy, or the freshly downloaded one
34+
/// @throws Exception if the download fails for a reason other than a transient 5xx
35+
static Path downloadIfMissing(Path tmp, Path cacheDir, URI url, String name) throws Exception {
36+
Path cached = cacheDir.resolve(name);
37+
if (Files.exists(cached)) {
38+
return cached;
39+
}
40+
Path dest = tmp.resolve(name);
41+
var conn = (HttpURLConnection) url.toURL().openConnection();
42+
int code = conn.getResponseCode();
43+
assumeTrue(code < 500, () -> "transient server error " + code + " for " + name);
44+
try (var in = conn.getInputStream()) {
45+
Files.copy(in, dest, StandardCopyOption.REPLACE_EXISTING);
46+
}
47+
return dest;
48+
}
49+
50+
/// Like [#downloadIfMissing] but skips the test (rather than failing it) on ANY download
51+
/// failure, not just a 5xx — for endpoints known to rate-limit or block by egress IP
52+
/// (e.g. CloudFront blocking CI runners with a 403) where any failure is infrastructure
53+
/// noise rather than a genuine "fixture is broken" signal.
54+
///
55+
/// @param tmp the test's `@TempDir`, used as the download destination when the
56+
/// fixture isn't already in `cacheDir`
57+
/// @param cacheDir persistent local cache directory
58+
/// @param url the fixture's download URL
59+
/// @param name the fixture's file name, used for both the cache lookup and dest file
60+
/// @return the local path to the fixture: the cached copy, or the freshly downloaded one
61+
static Path downloadIfMissingOrSkip(Path tmp, Path cacheDir, URI url, String name) {
62+
try {
63+
return downloadIfMissing(tmp, cacheDir, url, name);
64+
} catch (Exception e) {
65+
assumeTrue(false, "could not download " + name + ": " + e.getMessage());
66+
throw new AssertionError("unreachable");
67+
}
68+
}
69+
70+
/// Skips the running test (via `assumeTrue`) if `pingUrl` cannot be reached at all —
71+
/// distinguishes "no network" (skip, infrastructure) from "fixture missing" (fail, signal).
72+
///
73+
/// @param pingUrl a URL known to be reachable whenever the test network is up
74+
static void assumeNetworkAvailable(URI pingUrl) {
75+
try {
76+
pingUrl.toURL().openStream().close();
77+
} catch (Exception _) {
78+
assumeTrue(false, "no network");
79+
}
80+
}
81+
}

integration/src/test/java/io/github/dfa1/vortex/integration/ParquetImportIntegrationTest.java

Lines changed: 11 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,13 @@
1515
import org.junit.jupiter.api.Test;
1616
import org.junit.jupiter.api.io.TempDir;
1717

18-
import java.io.IOException;
1918
import java.net.URI;
2019
import java.nio.file.Files;
2120
import java.nio.file.Path;
22-
import java.nio.file.StandardCopyOption;
2321
import java.util.ArrayList;
2422
import java.util.List;
2523

2624
import static org.assertj.core.api.Assertions.assertThat;
27-
import static org.junit.jupiter.api.Assumptions.assumeTrue;
2825

2926
/// Round-trip: Parquet (via Hardwood) → Vortex (via ParquetImporter) → VortexReader.
3027
///
@@ -60,25 +57,14 @@ private static List<String> parquetColumnNames(Path path) throws Exception {
6057
}
6158

6259
private static Path download(Path tmp, String name) throws Exception {
63-
Path cached = Path.of("/tmp/parquet-fixtures", name);
64-
if (Files.exists(cached)) {
65-
return cached;
66-
}
67-
Path dest = tmp.resolve(name);
68-
try (var in = URI.create(FIXTURE_URL).toURL().openStream()) {
69-
Files.copy(in, dest, StandardCopyOption.REPLACE_EXISTING);
70-
}
71-
return dest;
60+
return LocalHttpCache.downloadIfMissing(tmp,
61+
Path.of("/tmp/parquet-fixtures"), URI.create(FIXTURE_URL), name);
7262
}
7363

7464
// ── helpers ──────────────────────────────────────────────────────────────
7565

7666
private static void assumeNetworkAvailable() {
77-
try {
78-
URI.create("https://raw.githubusercontent.com").toURL().openStream().close();
79-
} catch (Exception _) {
80-
assumeTrue(false, "no network");
81-
}
67+
LocalHttpCache.assumeNetworkAvailable(URI.create("https://raw.githubusercontent.com"));
8268
}
8369

8470
@Test
@@ -154,32 +140,20 @@ void stringColumnValuesMatch(@TempDir Path tmp) throws Exception {
154140
@Test
155141
void taxiParquet_importedSize_vsOriginal(@TempDir Path tmp) throws Exception {
156142
// Given — NYC Yellow Taxi 2024-01 (~3M rows, 19 cols, mix of I64 / F64 / I32 / Utf8).
157-
// Uses the same parquet file as ParquetVsVortexReadBenchmark (cached in /tmp).
158-
Path cached = Path.of("/tmp", "yellow_tripdata_2024-01.parquet");
159-
Path src;
160-
if (java.nio.file.Files.exists(cached)) {
161-
src = cached;
162-
} else {
163-
// CloudFront rate-limits / blocks some egress IPs (notably GitHub Actions
164-
// runners → 403). Skip rather than fail when the download isn't possible.
165-
src = tmp.resolve("yellow_tripdata_2024-01.parquet");
166-
try (var in = URI.create("https://d37ci6vzurychx.cloudfront.net/trip-data/yellow_tripdata_2024-01.parquet")
167-
.toURL().openStream()) {
168-
java.nio.file.Files.copy(in, src);
169-
} catch (IOException e) {
170-
org.junit.jupiter.api.Assumptions.assumeTrue(false,
171-
"could not download taxi parquet: " + e.getMessage());
172-
return;
173-
}
174-
}
143+
// Same fixture/cache path as TaxiParquetOracleVsJavaIntegrationTest. CloudFront
144+
// rate-limits/blocks some egress IPs (notably GitHub Actions runners → 403), so any
145+
// download failure skips rather than fails.
146+
Path src = LocalHttpCache.downloadIfMissingOrSkip(tmp, Path.of("/tmp"),
147+
URI.create("https://d37ci6vzurychx.cloudfront.net/trip-data/yellow_tripdata_2024-01.parquet"),
148+
"yellow_tripdata_2024-01.parquet");
175149
Path vortex = tmp.resolve("taxi.vortex");
176150

177151
// When
178152
ParquetImporter.importParquet(src, vortex);
179153

180154
// Then
181-
long parquetSize = java.nio.file.Files.size(src);
182-
long vortexSize = java.nio.file.Files.size(vortex);
155+
long parquetSize = Files.size(src);
156+
long vortexSize = Files.size(vortex);
183157
System.out.printf(
184158
"[TaxiSizeComparison] Parquet=%,d bytes (%.1f MB) Vortex=%,d bytes (%.1f MB) Vortex/Parquet=%.2fx%n",
185159
parquetSize, parquetSize / 1_048_576.0,

integration/src/test/java/io/github/dfa1/vortex/integration/PcoFixtureInspectionIntegrationTest.java

Lines changed: 6 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -12,21 +12,17 @@
1212
import org.junit.jupiter.api.io.TempDir;
1313

1414
import java.lang.foreign.MemorySegment;
15-
import java.net.HttpURLConnection;
1615
import java.net.URI;
1716
import java.nio.ByteBuffer;
1817
import java.nio.ByteOrder;
19-
import java.nio.file.Files;
2018
import java.nio.file.Path;
21-
import java.nio.file.StandardCopyOption;
2219
import java.util.ArrayList;
2320
import java.util.HashMap;
2421
import java.util.List;
2522
import java.util.Map;
2623
import java.util.TreeMap;
2724

2825
import static org.assertj.core.api.Assertions.assertThat;
29-
import static org.junit.jupiter.api.Assumptions.assumeTrue;
3026

3127
/// Phase 0 scoping for `vortex.pco` decode port.
3228
///
@@ -306,34 +302,16 @@ private static String formatDType(DType d) {
306302
return String.valueOf(d);
307303
}
308304

305+
// Reuse the version-keyed /tmp/pco-fixtures cache if present. The version segment
306+
// matters: the Rust reference rewrites identical file names with different bytes
307+
// across versions, so a version-less cache would serve stale bytes after a bump.
309308
private static Path downloadIfMissing(Path tmp, String name) throws Exception {
310-
// Reuse the version-keyed /tmp/pco-fixtures cache if present. The version
311-
// segment matters: the Rust reference rewrites identical file names with
312-
// different bytes across versions, so a version-less cache would serve
313-
// stale bytes after a bump.
314-
Path cached = Path.of("/tmp/pco-fixtures", FIXTURE_VERSION, name);
315-
if (Files.exists(cached)) {
316-
return cached;
317-
}
318-
Path dest = tmp.resolve(name);
319-
var conn = (HttpURLConnection) URI.create(BASE + name).toURL().openConnection();
320-
int code = conn.getResponseCode();
321-
// S3 occasionally returns a transient 5xx; that is infrastructure noise, not
322-
// an interop regression, so skip rather than redden the build. A 4xx (e.g. the
323-
// fixture was removed/renamed) is a genuine signal and still fails.
324-
assumeTrue(code < 500, () -> "transient S3 error " + code + " for " + name);
325-
try (var in = conn.getInputStream()) {
326-
Files.copy(in, dest, StandardCopyOption.REPLACE_EXISTING);
327-
}
328-
return dest;
309+
return LocalHttpCache.downloadIfMissing(tmp,
310+
Path.of("/tmp/pco-fixtures", FIXTURE_VERSION), URI.create(BASE + name), name);
329311
}
330312

331313
private static void assumeNetworkAvailable() {
332-
try {
333-
URI.create("https://vortex-compat-fixtures.s3.amazonaws.com").toURL().openStream().close();
334-
} catch (Exception _) {
335-
assumeTrue(false, "no network");
336-
}
314+
LocalHttpCache.assumeNetworkAvailable(URI.create("https://vortex-compat-fixtures.s3.amazonaws.com"));
337315
}
338316

339317
@Test

integration/src/test/java/io/github/dfa1/vortex/integration/RustJavaReaderComparisonIntegrationTest.java

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,6 @@
4141
import org.junit.jupiter.params.provider.ValueSource;
4242

4343
import java.net.URI;
44-
import java.net.http.HttpClient;
45-
import java.net.http.HttpRequest;
46-
import java.net.http.HttpResponse;
4744
import java.nio.file.Path;
4845
import java.util.LinkedHashMap;
4946
import java.util.Map;
@@ -59,25 +56,25 @@
5956
/// decode. A mismatch in any column value points to a decoding bug in the Java reader.
6057
class RustJavaReaderComparisonIntegrationTest {
6158

59+
private static final String FIXTURE_VERSION = "v0.75.0";
6260
private static final URI BASE =
63-
URI.create("https://vortex-compat-fixtures.s3.amazonaws.com/v0.75.0/arrays/");
61+
URI.create("https://vortex-compat-fixtures.s3.amazonaws.com/" + FIXTURE_VERSION + "/arrays/");
6462

6563
private static final Session SESSION = Session.create();
6664
private static final BufferAllocator ALLOCATOR = ArrowAllocation.rootAllocator();
67-
private static final HttpClient HTTP = HttpClient.newHttpClient();
6865

6966
static {
7067
NativeLoader.loadJni();
7168
}
7269

73-
private static Path download(URI uri, Path dir) throws Exception {
70+
private static Path download(URI uri, Path tmp) throws Exception {
7471
String name = uri.getPath().substring(uri.getPath().lastIndexOf('/') + 1);
75-
Path file = dir.resolve(name);
76-
HTTP.send(
77-
HttpRequest.newBuilder(uri).GET().build(),
78-
HttpResponse.BodyHandlers.ofFile(file)
79-
);
80-
return file;
72+
return LocalHttpCache.downloadIfMissing(tmp,
73+
Path.of("/tmp/rust-fixtures", FIXTURE_VERSION), uri, name);
74+
}
75+
76+
private static void assumeNetworkAvailable() {
77+
LocalHttpCache.assumeNetworkAvailable(URI.create("https://vortex-compat-fixtures.s3.amazonaws.com"));
8178
}
8279

8380
private static Stats rustStats(Path file) throws Exception {
@@ -403,6 +400,7 @@ private static Long stringByteLength(Array arr) {
403400
})
404401
void rust_vs_javaReader_statsMatch(String fixture, @TempDir Path tmp) throws Exception {
405402
// Given
403+
assumeNetworkAvailable();
406404
Path local = download(BASE.resolve(fixture), tmp);
407405
String inspect = VortexInspector.inspect(VortexReader.open(local));
408406
System.out.println(inspect);

integration/src/test/java/io/github/dfa1/vortex/integration/RustWritesJavaReadsIntegrationTest.java

Lines changed: 6 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -39,12 +39,9 @@
3939
import java.io.IOException;
4040
import java.lang.foreign.Arena;
4141
import java.lang.foreign.ValueLayout;
42-
import java.net.HttpURLConnection;
4342
import java.net.URI;
4443
import java.nio.ByteOrder;
45-
import java.nio.file.Files;
4644
import java.nio.file.Path;
47-
import java.nio.file.StandardCopyOption;
4845
import java.util.ArrayList;
4946
import java.util.Arrays;
5047
import java.util.HashMap;
@@ -54,7 +51,6 @@
5451
import java.util.function.Consumer;
5552

5653
import static org.assertj.core.api.Assertions.assertThat;
57-
import static org.junit.jupiter.api.Assumptions.assumeTrue;
5854

5955
/// Cross-compatibility: Rust (JNI) writer → Java reader.
6056
class RustWritesJavaReadsIntegrationTest {
@@ -220,33 +216,16 @@ private static long[] readJavaLongColumn(Path file, String column) throws IOExce
220216
}
221217
}
222218

219+
// Cache is keyed by fixture version: the Rust reference rewrites the same file names
220+
// with different bytes across versions, so a version-less cache would silently serve
221+
// stale bytes after a version bump.
223222
private static Path downloadIfMissing(Path tmp, String name) throws Exception {
224-
// Cache is keyed by fixture version: the Rust reference rewrites the same
225-
// file names with different bytes across versions, so a version-less cache
226-
// would silently serve stale bytes after a version bump.
227-
Path cached = Path.of("/tmp/pco-fixtures", FIXTURE_VERSION, name);
228-
if (Files.exists(cached)) {
229-
return cached;
230-
}
231-
Path dest = tmp.resolve(name);
232-
var conn = (HttpURLConnection) URI.create(S3_BASE + name).toURL().openConnection();
233-
int code = conn.getResponseCode();
234-
// S3 occasionally returns a transient 5xx; that is infrastructure noise, not
235-
// an interop regression, so skip rather than redden the build. A 4xx (e.g. the
236-
// fixture was removed/renamed) is a genuine signal and still fails.
237-
assumeTrue(code < 500, () -> "transient S3 error " + code + " for " + name);
238-
try (var in = conn.getInputStream()) {
239-
Files.copy(in, dest, StandardCopyOption.REPLACE_EXISTING);
240-
}
241-
return dest;
223+
return LocalHttpCache.downloadIfMissing(tmp,
224+
Path.of("/tmp/pco-fixtures", FIXTURE_VERSION), URI.create(S3_BASE + name), name);
242225
}
243226

244227
private static void assumeNetworkAvailable() {
245-
try {
246-
URI.create("https://vortex-compat-fixtures.s3.amazonaws.com").toURL().openStream().close();
247-
} catch (Exception _) {
248-
assumeTrue(false, "no network");
249-
}
228+
LocalHttpCache.assumeNetworkAvailable(URI.create("https://vortex-compat-fixtures.s3.amazonaws.com"));
250229
}
251230

252231
// ── S3 fixture round-trip: Rust-written pco → Java reader ────────────────

integration/src/test/java/io/github/dfa1/vortex/integration/TaxiParquetOracleVsJavaIntegrationTest.java

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,12 @@
1414

1515
import java.io.BufferedWriter;
1616
import java.io.IOException;
17+
import java.net.URI;
1718
import java.nio.file.Files;
1819
import java.nio.file.Path;
1920
import java.util.List;
2021

2122
import static org.assertj.core.api.Assertions.assertThat;
22-
import static org.junit.jupiter.api.Assumptions.assumeTrue;
2323

2424
/// Oracle vs SUT correctness test for the parquet importer.
2525
///
@@ -33,22 +33,27 @@
3333
/// value exactly.
3434
class TaxiParquetOracleVsJavaIntegrationTest {
3535

36-
private static final Path TAXI_PARQUET = Path.of("/tmp/yellow_tripdata_2024-01.parquet");
36+
// NYC Yellow Taxi 2024-01, same fixture/cache path as
37+
// ParquetImportIntegrationTest.taxiParquet_importedSize_vsOriginal.
38+
private static final String TAXI_URL =
39+
"https://d37ci6vzurychx.cloudfront.net/trip-data/yellow_tripdata_2024-01.parquet";
40+
private static final String TAXI_NAME = "yellow_tripdata_2024-01.parquet";
3741

3842
@Test
3943
void parquetImport_csvExport_matchesDirectParquetRead(@TempDir Path tmp) throws Exception {
4044
// Given
41-
assumeTrue(Files.exists(TAXI_PARQUET), "taxi parquet not cached in /tmp — skip");
45+
Path taxiParquet = LocalHttpCache.downloadIfMissingOrSkip(
46+
tmp, Path.of("/tmp"), URI.create(TAXI_URL), TAXI_NAME);
4247

4348
Path oracleCsv = tmp.resolve("oracle.csv");
4449
Path sutVortex = tmp.resolve("sut.vortex");
4550
Path sutCsv = tmp.resolve("sut.csv");
4651

4752
// When — oracle: hardwood → CSV
48-
writeOracleCsv(TAXI_PARQUET, oracleCsv);
53+
writeOracleCsv(taxiParquet, oracleCsv);
4954

5055
// When — SUT: parquet → vortex → CSV
51-
ParquetImporter.importParquet(TAXI_PARQUET, sutVortex);
56+
ParquetImporter.importParquet(taxiParquet, sutVortex);
5257
try (BufferedWriter writer = Files.newBufferedWriter(sutCsv)) {
5358
CsvExporter.exportCsv(sutVortex, writer, ExportOptions.defaults());
5459
}

0 commit comments

Comments
 (0)