mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-05 19:28:25 +00:00
fix(quartz): make the whole linuxX64 test suite pass; widen the CI leg
The 78 failures on this target were not 78 unimplemented actuals. Two root causes accounted for all of them. TestResourceLoader.linux was a TODO(), so every vector-driven suite failed before it reached any production code: the full MLS interop set, NIP-44, the NIP-01 hint indexer, the SQLite store's large-DB tests and the Bolt12 payer proofs — 69 tests. Implemented over platform.posix (linuxX64 has no Foundation for the Apple actual's NSData path), resolving against the same TEST_RESOURCES_ROOT that build.gradle.kts already exports onto every KotlinNativeTest task. The read is one ftell-sized allocation filled by fread, so a vector file costs exactly one ByteArray — less than the JVM actual's bufferedReader().readText(), which grows a StringBuilder as it goes. UriParser.linux never URL-decoded query values or fragments, though the JVM actual runs both through URLDecoder.decode(.., "UTF-8"). Every NIP-47 failure was one symptom of that: relay=wss%3A%2F%2Frelay.damus.io reached RelayUrlNormalizer still percent-encoded and came back "Invalid relay Url" (6 tests), and the deep-link round trips compared an encoded string against a plain one (3 tests). Added a decoder matching URLDecoder where the behaviour is observable — '+' to space, a run of consecutive %XX decoded as one UTF-8 sequence, malformed escapes throwing IllegalArgumentException — with the same short-circuit URLDecoder makes, returning the original instance when there is nothing to decode. Two other divergences fixed while there: getQueryParameter returned an empty list where the JVM returns null for an absent parameter, and the query string was re-split on every call rather than parsed once into a lazy map, so a URI read for four parameters was parsed four times. With those, linuxX64Test is 3495 tests, 0 failures, so the CI leg added alongside the LargeCache work drops its cache-package filter and runs the whole :quartz suite. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HxQ1QuyzSkR38iFHbREjoS
This commit is contained in:
+10
-22
@@ -194,24 +194,15 @@ jobs:
|
||||
name: geode Test Reports
|
||||
path: geode/build/reports
|
||||
|
||||
# linuxX64 is the only target whose LargeCache / ConcurrentHashCache actuals are
|
||||
# hand-written concurrent maps rather than a delegation to a platform concurrent
|
||||
# collection, and until this job existed nothing ran them: the target was compiled
|
||||
# by no CI leg at all. That is how a copy-on-write LargeCache with O(n) writes and a
|
||||
# non-atomic read-copy-write (concurrent writers silently dropped entries) sat in
|
||||
# the tree unnoticed.
|
||||
# Until this job existed nothing ran the linuxX64 target at all — it was compiled by
|
||||
# no CI leg. That is how a copy-on-write LargeCache with O(n) writes and a non-atomic
|
||||
# read-copy-write (concurrent writers silently dropped entries) sat in the tree
|
||||
# unnoticed, and how TestResourceLoader stayed a TODO() that failed every vector-driven
|
||||
# suite on the target.
|
||||
#
|
||||
# Scoped to the cache/concurrency packages on purpose. The full linuxX64Test suite
|
||||
# is 3,490 tests with 78 pre-existing failures, essentially all of them `TODO()`
|
||||
# stubs in linux actuals that were never written — MLS crypto, the SQLite driver,
|
||||
# NIP-44, Bolt12 — plus two URL-handling divergences. Filling those in is its own
|
||||
# project; gating PRs on them today would just mean a permanently red job. The
|
||||
# filter keeps the leg meaningful and green, and widening it is a one-line change
|
||||
# once the native actuals land.
|
||||
#
|
||||
# This still compiles and links the whole module for linuxX64, so a commonMain or
|
||||
# commonTest source that reaches for a JVM-only API fails here too — on a target
|
||||
# with no Foundation to fall back on the way Apple has.
|
||||
# Runs the whole :quartz suite on a Linux Native frontend, which also catches a
|
||||
# commonMain or commonTest source reaching for a JVM-only API on a target that, unlike
|
||||
# Apple, has no Foundation to fall back on.
|
||||
test-quartz-linux-native:
|
||||
needs: lint
|
||||
runs-on: ubuntu-latest
|
||||
@@ -242,11 +233,8 @@ jobs:
|
||||
key: konan-${{ runner.os }}-${{ hashFiles('gradle/libs.versions.toml') }}
|
||||
restore-keys: konan-${{ runner.os }}-
|
||||
|
||||
- name: Test Quartz caches on Linux Native
|
||||
run: |
|
||||
./gradlew :quartz:linuxX64Test \
|
||||
--tests "com.vitorpamplona.quartz.utils.cache.*" \
|
||||
--tests "com.vitorpamplona.quartz.utils.concurrent.*"
|
||||
- name: Test Quartz on Linux Native
|
||||
run: ./gradlew :quartz:linuxX64Test
|
||||
|
||||
- name: Linux Native Test Report
|
||||
uses: mikepenz/action-junit-report@a9170d5795813c01ab4901ffb045b52bab4ab09d # v6.5.0
|
||||
|
||||
@@ -121,41 +121,129 @@ actual class UriParser actual constructor(
|
||||
|
||||
actual fun path(): String? = parsedPath
|
||||
|
||||
actual fun queryParameterNames(): Set<String> {
|
||||
val query = parsedQuery ?: return emptySet()
|
||||
return query
|
||||
.split('&')
|
||||
.map { param ->
|
||||
val eqIndex = param.indexOf('=')
|
||||
if (eqIndex >= 0) param.substring(0, eqIndex) else param
|
||||
}.toSet()
|
||||
}
|
||||
/**
|
||||
* Parsed once and reused, mirroring the JVM actual's lazy map. The previous version
|
||||
* re-split the entire query string on every [getQueryParameter] call, so a URI read
|
||||
* for four parameters was parsed four times.
|
||||
*/
|
||||
private val queryParameters: Map<String, List<String>> by lazy {
|
||||
parsedQuery?.ifBlank { null }?.let { query ->
|
||||
val params = mutableMapOf<String, MutableList<String>>()
|
||||
|
||||
actual fun getQueryParameter(param: String): List<String>? {
|
||||
val query = parsedQuery ?: return null
|
||||
return query
|
||||
.split('&')
|
||||
.filter { part ->
|
||||
val eqIndex = part.indexOf('=')
|
||||
if (eqIndex >= 0) part.substring(0, eqIndex) == param else part == param
|
||||
}.map { part ->
|
||||
val eqIndex = part.indexOf('=')
|
||||
if (eqIndex >= 0) part.substring(eqIndex + 1) else ""
|
||||
query.split('&').forEach { paramValue ->
|
||||
val parts = paramValue.split("=", limit = 2)
|
||||
val currentValue =
|
||||
params.getOrPut(parts[0]) {
|
||||
mutableListOf()
|
||||
}
|
||||
|
||||
if (parts.size == 2) {
|
||||
currentValue.add(percentDecode(parts[1]))
|
||||
} else {
|
||||
currentValue.add("")
|
||||
}
|
||||
}
|
||||
|
||||
params
|
||||
} ?: emptyMap()
|
||||
}
|
||||
|
||||
val fragments: Map<String, String> by lazy {
|
||||
private val parsedFragments: Map<String, String> by lazy {
|
||||
parsedFragment?.ifBlank { null }?.let { keyValuePair ->
|
||||
keyValuePair.split('&').associate { paramValue ->
|
||||
val parts = paramValue.split("=", limit = 2)
|
||||
if (parts.size == 2) {
|
||||
parts[0] to parts[1]
|
||||
parts[0] to percentDecode(parts[1])
|
||||
} else {
|
||||
parts[0] to ""
|
||||
parts[0] to "" // Handle parameters without a value
|
||||
}
|
||||
}
|
||||
} ?: emptyMap()
|
||||
}
|
||||
|
||||
actual fun fragments(): Map<String, String> = fragments
|
||||
actual fun queryParameterNames(): Set<String> = queryParameters.keys
|
||||
|
||||
/** Null — not an empty list — when the parameter is absent, as on the JVM. */
|
||||
actual fun getQueryParameter(param: String): List<String>? = queryParameters[param]
|
||||
|
||||
actual fun fragments(): Map<String, String> = parsedFragments
|
||||
}
|
||||
|
||||
/**
|
||||
* `java.net.URLDecoder.decode(value, "UTF-8")` — which is literally what the JVM actual
|
||||
* calls — for a target with no `java.net`.
|
||||
*
|
||||
* This is the whole reason NIP-47 failed on linuxX64: the parser returned query values
|
||||
* exactly as they appeared in the URI, so `relay=wss%3A%2F%2Frelay.damus.io` reached
|
||||
* `RelayUrlNormalizer` still percent-encoded and came back "Invalid relay Url". Decoding
|
||||
* belongs here rather than in each caller, because the JVM and Apple actuals both hand
|
||||
* back decoded values and common code is written against that.
|
||||
*
|
||||
* Matches `URLDecoder` in the details that are observable: `+` becomes a space, a run of
|
||||
* consecutive `%XX` is decoded as one UTF-8 sequence (so multi-byte characters survive),
|
||||
* every other character passes through, and a malformed escape throws
|
||||
* [IllegalArgumentException] rather than being silently kept — the same failure the JVM
|
||||
* gives for the same input.
|
||||
*/
|
||||
private fun percentDecode(value: String): String {
|
||||
// The short-circuit URLDecoder also makes: with nothing to change, return the
|
||||
// original instance rather than rebuilding it. Most query values hit this.
|
||||
if (value.indexOf('%') < 0 && value.indexOf('+') < 0) return value
|
||||
|
||||
val result = StringBuilder(value.length)
|
||||
var index = 0
|
||||
// Sized on first use for the longest run that could still follow, then reused —
|
||||
// one allocation for the whole string, as in URLDecoder.
|
||||
var escaped: ByteArray? = null
|
||||
|
||||
while (index < value.length) {
|
||||
when (val char = value[index]) {
|
||||
'+' -> {
|
||||
result.append(' ')
|
||||
index++
|
||||
}
|
||||
|
||||
'%' -> {
|
||||
val buffer = escaped ?: ByteArray((value.length - index) / 3).also { escaped = it }
|
||||
var count = 0
|
||||
while (index + 2 < value.length && value[index] == '%') {
|
||||
buffer[count++] = decodeEscape(value, index)
|
||||
index += 3
|
||||
}
|
||||
if (index < value.length && value[index] == '%') {
|
||||
throw IllegalArgumentException("URLDecoder: Incomplete trailing escape (%) pattern")
|
||||
}
|
||||
result.append(buffer.decodeToString(0, count))
|
||||
}
|
||||
|
||||
else -> {
|
||||
result.append(char)
|
||||
index++
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
return result.toString()
|
||||
}
|
||||
|
||||
private fun decodeEscape(
|
||||
value: String,
|
||||
index: Int,
|
||||
): Byte {
|
||||
val high = hexDigit(value[index + 1])
|
||||
val low = hexDigit(value[index + 2])
|
||||
if (high < 0 || low < 0) {
|
||||
throw IllegalArgumentException(
|
||||
"URLDecoder: Illegal hex characters in escape (%) pattern - ${value.substring(index, index + 3)}",
|
||||
)
|
||||
}
|
||||
return ((high shl 4) or low).toByte()
|
||||
}
|
||||
|
||||
private fun hexDigit(char: Char): Int =
|
||||
when (char) {
|
||||
in '0'..'9' -> char - '0'
|
||||
in 'a'..'f' -> char - 'a' + 10
|
||||
in 'A'..'F' -> char - 'A' + 10
|
||||
else -> -1
|
||||
}
|
||||
|
||||
@@ -20,12 +20,82 @@
|
||||
*/
|
||||
package com.vitorpamplona.quartz
|
||||
|
||||
actual class TestResourceLoader actual constructor() {
|
||||
actual fun loadDecompressString(file: String): String {
|
||||
TODO("Not yet implemented")
|
||||
}
|
||||
import com.vitorpamplona.quartz.utils.GZip
|
||||
import kotlinx.cinterop.ExperimentalForeignApi
|
||||
import kotlinx.cinterop.addressOf
|
||||
import kotlinx.cinterop.convert
|
||||
import kotlinx.cinterop.toKString
|
||||
import kotlinx.cinterop.usePinned
|
||||
import platform.posix.SEEK_END
|
||||
import platform.posix.SEEK_SET
|
||||
import platform.posix.fclose
|
||||
import platform.posix.fopen
|
||||
import platform.posix.fread
|
||||
import platform.posix.fseek
|
||||
import platform.posix.ftell
|
||||
import platform.posix.getenv
|
||||
|
||||
actual fun loadString(file: String): String {
|
||||
TODO("Not yet implemented")
|
||||
/**
|
||||
* Linux/Native actual for [TestResourceLoader].
|
||||
*
|
||||
* Until this existed it was `TODO()`, which failed 69 tests on this target — every
|
||||
* suite driven by a vector file: the whole MLS interop set, NIP-44, the NIP-01 hint
|
||||
* indexer, the SQLite store's large-DB tests and the Bolt12 payer proofs. None of them
|
||||
* were failing because of missing production code; they could not read their input.
|
||||
*
|
||||
* Resolves paths against `TEST_RESOURCES_ROOT`, the same environment variable the Apple
|
||||
* actual uses, exported onto every `KotlinNativeTest` task by `quartz/build.gradle.kts`.
|
||||
* Reads through `platform.posix` rather than Foundation, which linuxX64 does not have.
|
||||
*
|
||||
* The read is a single `stat`-sized allocation filled by `fread`, so a vector file
|
||||
* costs exactly one `ByteArray` — less than the JVM actual's `bufferedReader().readText()`,
|
||||
* which grows a `StringBuilder` as it goes.
|
||||
*/
|
||||
@OptIn(ExperimentalForeignApi::class)
|
||||
actual class TestResourceLoader actual constructor() {
|
||||
actual fun loadDecompressString(file: String): String = GZip.decompress(readBytes(file))
|
||||
|
||||
actual fun loadString(file: String): String = readBytes(file).decodeToString()
|
||||
|
||||
private fun readBytes(file: String): ByteArray {
|
||||
val root =
|
||||
getenv("TEST_RESOURCES_ROOT")?.toKString()
|
||||
?: throw IllegalStateException(
|
||||
"TEST_RESOURCES_ROOT is not set. quartz/build.gradle.kts exports it onto every " +
|
||||
"KotlinNativeTest task; running the test binary directly has to set it too.",
|
||||
)
|
||||
|
||||
val path = "$root/$file"
|
||||
val handle = fopen(path, "rb") ?: throw IllegalArgumentException("Resource not found: $path")
|
||||
|
||||
try {
|
||||
if (fseek(handle, 0, SEEK_END) != 0) throw IllegalArgumentException("Resource is not seekable: $path")
|
||||
val size = ftell(handle)
|
||||
if (size < 0L) throw IllegalArgumentException("Cannot determine the size of: $path")
|
||||
if (size == 0L) return ByteArray(0)
|
||||
if (fseek(handle, 0, SEEK_SET) != 0) throw IllegalArgumentException("Cannot rewind: $path")
|
||||
|
||||
val bytes = ByteArray(size.toInt())
|
||||
bytes.usePinned { pinned ->
|
||||
var read = 0
|
||||
while (read < bytes.size) {
|
||||
val count =
|
||||
fread(
|
||||
pinned.addressOf(read),
|
||||
1.convert(),
|
||||
(bytes.size - read).convert(),
|
||||
handle,
|
||||
).toInt()
|
||||
if (count <= 0) break
|
||||
read += count
|
||||
}
|
||||
if (read != bytes.size) {
|
||||
throw IllegalArgumentException("Short read on $path: got $read of ${bytes.size} bytes")
|
||||
}
|
||||
}
|
||||
return bytes
|
||||
} finally {
|
||||
fclose(handle)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user