Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion .github/workflows/jtreg.yml
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,10 @@ jobs:
release=$(sed -n 's/^release=//p' scripts/build-jdk.sh)
git clone -q --filter=blob:none --sparse --depth 1 --branch "jdk-${release%+*}-ga" https://github.com/openjdk/jdk25u.git build/ref/jdk25u
dirs=$(grep -v '^#' test/jtreg/passing.txt | cut -d/ -f1-2 | sort -u | sed 's|^|test/jdk/|')
git -C build/ref/jdk25u sparse-checkout set test/lib test/jdk/lib test/jdk/tools/lib $dirs
git -C build/ref/jdk25u sparse-checkout set test/lib test/jdk/lib test/jdk/tools/lib \
src/java.base/share/data/lsrdata src/java.base/share/data/unicodedata $dirs
# the @library directories of the listed tests, which can lie outside those
git -C build/ref/jdk25u sparse-checkout add $(node scripts/jtreg/run.ts build/ref/jdk25u --only-passing --library-dirs)
read -ra filters <<< "$FILTERS"
node scripts/jtreg/run.ts build/ref/jdk25u --only-passing --jobs 4 --timeout 600 --vm dist/node.js \
--shard ${{ matrix.shard }}/${{ strategy.job-total }} "${filters[@]}"
Expand Down
24 changes: 22 additions & 2 deletions docs/development.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,28 @@ node scripts/jtreg/run.ts <jdk-source> java/lang

`<jdk-source>` is a checkout of `openjdk/jdk25u` at the pinned JDK's tag:
`jdk-<version>-ga`, where `<version>` is `release=` in `scripts/build-jdk.sh`
without the `+build` part. A sparse checkout of `test/lib` and the test
directories is enough.
without the `+build` part. A sparse checkout of `test/lib`, the test
directories, the `@library` directories they name and the Unicode and language
data under `src/java.base/share/data` (`lsrdata`, `unicodedata`) is enough;
`--only-passing --library-dirs` prints the `@library` directories of the listed
tests.

The runner follows jtreg where tests rely on it: `test.src.path` holds the test
directory and its `@library` directories, `${test.main.class}`, `${test.src}`
and the like are replaced in `@run` lines, the test directory is on the run
class path when the test's package starts there (for resource files next to a
test), `@clean` removes the named
classes between actions, `@run driver` runs as `main`, and `@run build` is a
`@build`. A test that patches `java.base`, needs a native library, is manual or
runs in a named module is skipped.

HotSpot always has every locale, but java-web loads the locale data only with
`--locale-data`, which makes start-up and locale-heavy tests slower (about 15%
on the `java/text`, `java/util/Locale`, `java/util/Calendar` and
`java/time/format` tests, and three tests over 5 times slower). So the runner
passes it only to the tests in `test/jtreg/locale-data.txt`. To find which new
tests need it, run them with and without the runner's `--locale-data` flag,
which sets it for every test.

The runner evaluates a test's `@requires` as jtreg does on a release Linux
HotSpot with default options (for example `vm.continuations` is true and
Expand Down
120 changes: 103 additions & 17 deletions scripts/jtreg/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,31 @@
//
// node scripts/jtreg/run.ts <jdk-source-checkout> [dir or file filters...]
// [--jobs 1] [--timeout 120] [--rerun] [--only-passing] [--shard 1/4] [--vm path/to/node.ts] [--cool 75]
// [--locale-data] [--library-dirs]
//
// Each test is compiled the way jtreg does it, run on HotSpot first (only
// tests that pass there count), then on java-web in a child process.
// Results go to build/jtreg/results.jsonl; --only-passing reruns the tests
// listed in test/jtreg/passing.txt (those under the filters, if any) on
// java-web alone, and fails if any of them regress. --shard k/n runs the kth
// of n shards with about equal run time, for parallel CI jobs.
// of n shards with about equal run time, for parallel CI jobs. Tests listed in
// test/jtreg/locale-data.txt start java-web with --locale-data (every locale
// costs start-up and run time, so the others run without it).

import { createHash } from "node:crypto";
import { spawn, spawnSync } from "node:child_process";
import type { ChildProcess } from "node:child_process";
import { appendFileSync, existsSync, mkdirSync, readFileSync, readdirSync, realpathSync, rmSync, statSync, writeFileSync } from "node:fs";
import { totalmem } from "node:os";
import { dirname, join, relative, resolve } from "node:path";
import { release, totalmem } from "node:os";
import { dirname, join, normalize, relative, resolve } from "node:path";
import { hotspot } from "../hotspot.ts";

const root = join(import.meta.dirname, "..", "..");
const out = join(root, "build", "jtreg");
const resultsFile = join(out, "results.jsonl");
const passingFile = join(root, "test", "jtreg", "passing.txt");
// tests that need the locales other than English (java-web's --locale-data)
const localeFile = join(root, "test", "jtreg", "locale-data.txt");
// how long each listed test takes on java-web, for splitting them into shards
const timesFile = join(root, "test", "jtreg", "times.tsv");

Expand All @@ -41,6 +46,9 @@ const rerun = has("--rerun");
// pause new tests while the CPU is at or above this many degrees C (for small boards)
const cool = Number(flag("--cool", "0"));
const onlyPassing = has("--only-passing");
// --locale-data runs every test with the locale data, to find the tests that need it
const localeData = has("--locale-data");
const printLibraryDirs = has("--library-dirs");
const [shardK, shardN] = flag("--shard", "1/1").split("/").map(Number);
if (!Number.isInteger(shardK) || !Number.isInteger(shardN) || shardK < 1 || shardK > shardN) {
console.error("--shard takes k/n with 1 <= k <= n");
Expand All @@ -62,6 +70,12 @@ const javaHome = dirname(dirname(realpathSync(hotspot("java"))));
// property not listed makes the test skip.
const required: Record<string, string> = {
"os.family": "linux",
"os.name": "Linux",
"os.version": release(),
"os.simpleArch": process.arch === "arm64" ? "aarch64" : process.arch,
"jdk.static": "false",
"java.enablePreview": "false",
"java.runtime.name": "OpenJDK Runtime Environment",
"os.arch": process.arch === "arm64" ? "aarch64" : process.arch === "x64" ? "amd64" : process.arch,
"os.maxMemory": String(totalmem()),
"sun.arch.data.model": "64",
Expand Down Expand Up @@ -139,8 +153,8 @@ function requiresHolds(expr: string): boolean {
return v;
}

type Kind = "main" | "testng" | "junit" | "compile";
// a compile action keeps its javac options in opts and its files in args
type Kind = "main" | "testng" | "junit" | "compile" | "clean";
// a compile action keeps its javac options in opts and its files in args; a clean action its class names in args
interface Action { kind: Kind; opts: string[]; main: string; args: string[]; timeout: number }
interface TestCase {
id: string;
Expand Down Expand Up @@ -232,7 +246,12 @@ function parseFile(file: string): TestCase[] {
}
break;
case "ignore": tc.skip ??= "ignore"; break;
case "clean": tc.skip ??= "@clean"; break;
case "clean": {
// before any other action there is nothing to clean
const names = value.split(/\s+/).filter(Boolean);
if (tc.runs.length && names.length) tc.runs.push({ kind: "clean", opts: [], main: "", args: names, timeout: 120 });
break;
}
case "compile": {
if (framework) break;
if (mods.some((x) => !x.startsWith("timeout="))) { tc.skip ??= `@${tag}`; break; }
Expand All @@ -251,9 +270,15 @@ function parseFile(file: string): TestCase[] {
}
case "run": {
if (framework) break;
const m = value.match(/^(\S+)\s*(.*)$/);
const m = value.replaceAll("${test.main.class}", base).match(/^(\S+)\s*(.*)$/);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only ${test.main.class} is replaced before parsing the @run line. If a test writes @run main ${test.src}/MyClass, the variable remains in the first word and the main class becomes the literal string ${test.src}/MyClass. The expand function runs later on a.args and a.opts but not on a.main (the main class extraction logic is not fully visible in the diff).

if (!m) break;
const kind = m[1].split("/");
// a driver runs in jtreg's own VM; on its own it is a main
if (kind[0] === "driver") kind[0] = "main";
if (kind[0] === "build") {
tc.build.push(...m[2].split(/\s+/).filter(Boolean));
break;
}
if (kind[0] !== "main" && kind[0] !== "testng" && kind[0] !== "junit") { tc.skip ??= `@run ${kind[0]}`; break; }
if (kind.includes("native") || kind.includes("manual")) { tc.skip ??= `@run ${m[1]}`; break; }
const t = kind.find((k) => k.startsWith("timeout="));
Expand All @@ -278,7 +303,7 @@ function parseFile(file: string): TestCase[] {
const hasAction = tags.some(([t]) => /^(run|compile|build|clean|ignore)(\/|$)/.test(t));
if (framework) tc.runs = [{ kind: framework, opts: [], main: base, args: [], timeout: 120 }];
else if (!hasAction) tc.runs.push({ kind: "main", opts: [], main: base, args: [], timeout: 120 });
else if (!tc.runs.some((r) => r.kind !== "compile")) tc.skip ??= "no run action";
else if (!tc.runs.some((r) => r.kind !== "compile" && r.kind !== "clean")) tc.skip ??= "no run action";
cases.push(tc);
}
return cases;
Expand Down Expand Up @@ -404,26 +429,59 @@ async function compile(tc: TestCase, cp: string[]): Promise<string | null> {
}
}
}
// a main class from a library, as jtreg builds it implicitly
for (const r of tc.runs) {
if (r.kind !== "main" || r.main.includes("/")) continue;
for (const base of libs) {
const f = join(base, r.main.replace(/\./g, "/") + ".java");
if (existsSync(f)) { files.push(f); break; }
}
}
return javacInto(tc, cp, [], [...new Set(files)]);
}

async function javacInto(tc: TestCase, cp: string[], opts: string[], files: string[]): Promise<string | null> {
const libs = tc.libraries.map((l) => libraryDir(tc, l));
const args = ["-d", classesDir(tc), "-nowarn", "-XDignore.symbol.file", "-sourcepath", [packageRoot(tc), ...libs].join(":"),
"-cp", cp.join(":"), ...exportsFlags(tc), ...(tc.preview ? ["--enable-preview", "--release", "25"] : []), ...opts, ...files];
"-cp", cp.join(":"), ...exportsFlags(tc), ...(tc.preview ? ["--enable-preview", "-source", "25"] : []), ...opts, ...files];
const [rc, msg] = (await javac(args)).split("\t");
return rc === "0" ? null : msg;
}

function props(tc: TestCase): string[] {
const dest = classesDir(tc);
const srcPath = [tc.dir, ...tc.libraries.map((l) => libraryDir(tc, l))].join(":");
return [
`-Dtest.src=${tc.dir}`, `-Dtest.src.path=${tc.dir}`, `-Dtest.classes=${dest}`, `-Dtest.class.path=${dest}`,
`-Dtest.src=${tc.dir}`, `-Dtest.src.path=${srcPath}`, `-Dtest.classes=${dest}`, `-Dtest.class.path=${dest}`,
`-Dtest.jdk=${javaHome}`, `-Dcompile.jdk=${javaHome}`, `-Dtest.root=${testRoot}`, "-Dtest.vm.opts=", "-Dtest.java.opts=",
`-Dtest.file=${tc.file}`, `-Dtest.name=${tc.id}`,
];
}

// jtreg's ${...} in @run words (${test.main.class} is already in place)
function expand(tc: TestCase, word: string): string {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expand function handles only test.src, test.classes, test.jdk, compile.jdk, test.root. jtreg also defines test.src.path, test.class.path, test.file, test.name; tests using these in @run arguments will not have them expanded. Since they are also passed as -D properties the test code can read them, but the command-line substitution is incomplete.

const dest = classesDir(tc);
const values: Record<string, string> = {
"test.src": tc.dir, "test.classes": dest, "test.jdk": javaHome, "compile.jdk": javaHome, "test.root": testRoot,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expand function handles only 5 of the jtreg variables. jtreg also defines test.src.path, test.class.path, test.file, and test.name; tests using these in @run arguments will not have them expanded. They are passed as -D properties (line 454-458) so test code can read them, but @run argument expansion is incomplete.

};
return word.replace(/\$\{([\w.]+)\}/g, (all, name) => values[name] ?? all);
}

// jtreg's @clean: the named classes and their nested classes
function clean(tc: TestCase, names: string[]) {
const dest = classesDir(tc);
for (const name of names) {
const path = join(dest, name.replace(/\.\*$/, "").replace(/\./g, "/"));
if (name.endsWith(".*") || name === "*") {
for (const f of existsSync(path) ? readdirSync(path) : []) if (f.endsWith(".class")) rmSync(join(path, f));
} else {
rmSync(path + ".class", { force: true });
const dir = dirname(path), base = path.slice(dir.length + 1);
for (const f of existsSync(dir) ? readdirSync(dir) : []) if (f.startsWith(base + "$") && f.endsWith(".class")) rmSync(join(dir, f));
}
}
}

interface RunResult { ok: boolean; ms: number; timedOut: boolean; tail: string }

function exec(cmd: string, args: string[], cwd: string, limit: number): Promise<RunResult> {
Expand Down Expand Up @@ -451,6 +509,7 @@ function exec(cmd: string, args: string[], cwd: string, limit: number): Promise<
async function runCase(tc: TestCase): Promise<Record<string, unknown>> {
if (tc.skip) return { status: "skip", why: tc.skip };
const dest = classesDir(tc);
rmSync(dest, { recursive: true, force: true });
const path = [dest, ...(usesFramework(tc) ? await frameworkPath() : [])];
const err = await compile(tc, path);
if (err) return { status: "skip", why: `javac: ${err}` };
Expand All @@ -462,32 +521,43 @@ async function runCase(tc: TestCase): Promise<Record<string, unknown>> {
return dir;
};
const preview = tc.preview ? ["--enable-preview"] : [];
const cp = ["-cp", path.join(":")];
// resource files next to a test; a package root above the test dir holds other tests' META-INF
const cp = ["-cp", [...path, ...(packageRoot(tc) === tc.dir ? [tc.dir] : [])].join(":")];
const entry = (a: Action) => a.kind === "main" ? [a.main] : ["TestRunner", a.kind, a.main];
const words = (a: Action) => [...a.opts, ...props(tc)].map((w) => expand(tc, w));
const argsOf = (a: Action) => a.args.map((w) => expand(tc, w));
// the whole sequence on one VM, then the other: later runs may depend on files earlier ones left
let refMs = 0;
let work = scratch("hotspot");
// tests in passing.txt already pass on HotSpot
for (const a of onlyPassing ? [] : tc.runs) {
if (a.kind === "clean") { clean(tc, a.args); continue; }
if (a.kind === "compile") {
const err = await javacInto(tc, path, a.opts, a.args.map((f) => resolve(tc.dir, f)));
if (err) return { status: "skip", why: `javac: ${err}` };
continue;
}
const ref = await exec(hotspot("java"), [...preview, ...exportsFlags(tc), ...a.opts, ...props(tc), ...cp, ...entry(a), ...a.args], work, a.timeout);
const ref = await exec(hotspot("java"), [...preview, ...exportsFlags(tc), ...words(a), ...cp, ...entry(a), ...argsOf(a)], work, a.timeout);
if (!ref.ok) return { status: "skip", why: `fails on HotSpot: ${ref.tail}` };
refMs += ref.ms;
}
let total = 0;
work = scratch("java-web");
if (!onlyPassing && tc.runs.some((a) => a.kind === "clean")) {
// the HotSpot sequence cleaned classes the first action needs
rmSync(classesDir(tc), { recursive: true, force: true });
const err = await compile(tc, path);
if (err) return { status: "skip", why: `javac: ${err}` };
}
for (const a of tc.runs) {
if (a.kind === "clean") { clean(tc, a.args); continue; }
// recompile, since a later @compile may have replaced classes the first run used
if (a.kind === "compile") {
const err = await javacInto(tc, path, a.opts, a.args.map((f) => resolve(tc.dir, f)));
if (err) return { status: "skip", why: `javac: ${err}` };
continue;
}
const ours = await exec("node", [vm, "--jdk", join(root, "build", "jdk"), ...preview, ...exportsFlags(tc), ...a.opts, ...props(tc), ...cp, ...entry(a), ...a.args], work, timeout);
const ours = await exec("node", [vm, "--jdk", join(root, "build", "jdk"), ...(localeData || needsLocale.has(tc.id) ? ["--locale-data"] : []), ...preview, ...exportsFlags(tc), ...words(a), ...cp, ...entry(a), ...argsOf(a)], work, timeout);
total += ours.ms;
if (!ours.ok) return { status: ours.timedOut ? "timeout" : "fail", ms: total, refMs, why: ours.tail };
}
Expand Down Expand Up @@ -535,11 +605,13 @@ function shard(tests: TestCase[]): TestCase[] {
return tests.filter((tc) => mine.has(tc));
}

// test ids in passing.txt
function listed(): string[] {
if (!existsSync(passingFile)) return [];
return readFileSync(passingFile, "utf8").split("\n").map((l) => l.trim()).filter((l) => l && !l.startsWith("#"));
// test ids in a list file
function readList(file: string): string[] {
if (!existsSync(file)) return [];
return readFileSync(file, "utf8").split("\n").map((l) => l.trim()).filter((l) => l && !l.startsWith("#"));
}
const listed = () => readList(passingFile);
const needsLocale = new Set(readList(localeFile));

mkdirSync(out, { recursive: true });
let selected: TestCase[];
Expand All @@ -559,6 +631,20 @@ if (onlyPassing) {
return ids.has(tc.id) || whole.has(file) || !narrowed.has(file);
});
}
if (printLibraryDirs) {
// the checkout directories the selected tests' @library lines name, for a sparse checkout
const dirs = new Set<string>();
for (const tc of selected) {
for (const lib of tc.libraries) {
if (lib.startsWith("/")) dirs.add(`test/jdk${lib}`).add(lib.slice(1));

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For an absolute @library path like /test/lib, this adds both test/jdk/test/lib (correct, relative to test/jdk root) and test/lib (incorrect, relative to repo root). The latter is already in the sparse checkout from the workflow's static list, so it only causes a redundant entry, but the logic is wrong.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For an absolute @library path like /test/lib, this adds both test/jdk/test/lib (correct, relative to test/jdk root) and test/lib (incorrect, relative to repo root). The latter is already in the sparse checkout from the workflow's static list, so it only causes a redundant entry, but the logic is wrong. Use an if/else or two separate add calls with the correct paths.

else dirs.add(relative(src, resolve(tc.dir, lib)));
}
}
const whole = new Set(["", ".", "test", "test/jdk"]);
const found = [...dirs].map((d) => normalize(d).replace(/\/$/, "")).filter((d) => !d.startsWith("..") && !whole.has(d));
console.log([...new Set(found)].sort().join("\n"));
process.exit(0);
}
selected = shard(selected);
const done = new Set<string>();
if (!rerun && !onlyPassing && existsSync(resultsFile)) {
Expand Down
56 changes: 56 additions & 0 deletions test/jtreg/locale-data.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# tests that run with --locale-data (see docs/development.md)
java/io/PrintStream/FormatLocale.java
java/lang/invoke/LFInterpret/ReflectionInInterpretTest.java
java/text/BreakIterator/NewVSOld_th_TH.java
java/text/Collator/APITest.java
java/text/Collator/CollationKeyTest.java
java/text/Collator/JapaneseTest.java
java/text/Collator/KoreanTest.java
java/text/Format/CompactNumberFormat/TestCompactNumber.java
java/text/Format/CompactNumberFormat/TestFormatToCharacterIterator.java
java/text/Format/CompactNumberFormat/TestParseBigDecimal.java
java/text/Format/CompactNumberFormat/TestUExtensionOverride.java
java/text/Format/DateFormat/Bug6683975.java
java/text/Format/DateFormat/Bug8139572.java
java/text/Format/DateFormat/ContextMonthNamesTest.java
java/text/Format/DateFormat/LocaleDateFormats.java
java/text/Format/DateFormat/NonGregorianFormatTest.java
java/text/Format/DateFormat/bug4117335.java
java/text/Format/ListFormat/TestListFormat.java
java/text/Format/ListFormat/ToStringTest.java
java/text/Format/MessageFormat/LargeMessageFormat.java
java/text/Format/NumberFormat/Bug8132125.java
java/text/Format/NumberFormat/CurrencyFormat.java
java/text/Format/NumberFormat/IntlTestNumberFormatAPI.java
java/text/Format/NumberFormat/MultipleNumberScriptTest.java
java/text/Format/NumberFormat/NumberTest.java
java/time/test/java/time/chrono/TestEraDisplayName.java
java/time/test/java/time/format/TestDateTimeFormatterBuilder.java
java/time/test/java/time/format/TestDateTimeFormatterBuilderWithLocale.java
java/time/test/java/time/format/TestDateTimeTextProviderWithLocale.java
java/time/test/java/time/format/TestLocalizedOffsetPrinterParser.java
java/time/test/java/time/format/TestNarrowMonthNamesAndDayNames.java
java/time/test/java/time/format/TestTextPrinterWithLocale.java
java/time/test/java/time/format/TestUnicodeExtension.java
java/util/Calendar/CldrFormatNamesTest.java
java/util/Calendar/JapaneseCalendarNameTest.java
java/util/Calendar/NarrowNamesTest.java
java/util/Currency/CurrencyTest.java
java/util/Formatter/FormatLocale.java
java/util/Locale/AliasesShouldBeRecognizedInCLDR.java
java/util/Locale/HashCodeTest.java
java/util/Locale/JDK7LocaleServiceDiffs.java
java/util/Locale/LocaleTest.java
java/util/Locale/ThaiGov.java
java/util/Locale/UserRegionTest.java
java/util/Locale/bcp47u/CurrencyFormatTests.java
java/util/Locale/bcp47u/CurrencyTests.java
java/util/Locale/bcp47u/DisplayNameTests.java
java/util/Locale/bcp47u/FormatTests.java
java/util/Locale/bcp47u/SymbolsTests.java
java/util/Scanner/ScanTest.java
java/util/Scanner/spi/UseLocaleWithProvider.java
java/util/TimeZone/Bug8149452.java
java/util/TimeZone/Bug8167143.java
java/util/TimeZone/CLDRDisplayNamesTest.java
java/util/TimeZone/HongKong.java
Loading
Loading