From 1a11cc112006e68f2f6f964da48f031ef79209a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 3 Sep 2026 22:02:34 +0000 Subject: [PATCH] =?UTF-8?q?Dire=20dans=20la=20configuration=20O=C3=99=20se?= =?UTF-8?q?rvir,=20et=20rendre=20=C3=A0=20la=20ligne=20de=20commande=20sa?= =?UTF-8?q?=20priorit=C3=A9?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Répéter --serve-host à chaque lancement, c'est finir par ne plus le répéter : l'interface qu'une machine expose est une propriété de cette machine, donc SERVE_HOST se lit dans le fichier de configuration. Ce qui n'y est pas, et ne doit pas y être, c'est la décision de servir. La clé dit où écouter quand --serve est donné, et ne met jamais rien en écoute d'elle-même : un fichier de configuration voyage — dans un dépôt, dans un ticket, sur une autre machine — et un fichier capable d'ouvrir un port en voyageant serait illisible en confiance. Un test tient la ligne : un seul « serve = true » dans le switch des options. Au-delà de la boucle locale, le rapport devient lisible et annotable par qui atteint le port, avec les valeurs capturées d'une vraie application dedans. Le défaut reste donc 127.0.0.1, la clé est livrée commentée avec son prix à côté, et la documentation porte le déploiement qui le fait proprement : le proxy termine le TLS et l'outil ne répond qu'à lui. En l'ajoutant, un défaut est apparu. merge() était une liste écrite à la main, et elle couvrait neuf réglages sur vingt-deux. Tout le reste donné sur la ligne était abandonné sans un mot dès qu'un --config l'accompagnait : --level coverage mesurait tout, et --jacoco-reports data écrivait ses cent quatre-vingts fichiers. Rien n'échouait — l'exécution faisait simplement autre chose que ce qu'on lui avait demandé, et le seul moyen de s'en apercevoir était de compter les fichiers après coup. Le fichier trouvé par son nom, sans --config, était pire encore : il remplaçait tout. La règle est maintenant appliquée plutôt qu'énumérée — un réglage qui diffère d'une Config neuve est un réglage posé par la ligne. Une liste doit être maintenue à chaque nouvelle option ; une comparaison non. Le test parcourt les champs par réflexion et casse la construction sur celui qui ne passerait pas. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01J956wjynbd7fkZx4HpjHzP --- CLAUDE.md | 30 ++++ bin/acceptance-local.sh | 47 ++++++ docs/outil/annotations.md | 69 ++++++++ docs/outil/mode-emploi.md | 2 +- .../src/main/java/lab/xray/Config.java | 36 ++++- orchestrator/src/main/java/lab/xray/Main.java | 63 ++++++-- .../src/test/java/lab/xray/ConfigTest.java | 21 +++ .../java/lab/xray/SettingsPrecedenceTest.java | 150 ++++++++++++++++++ 8 files changed, 394 insertions(+), 24 deletions(-) create mode 100644 orchestrator/src/test/java/lab/xray/SettingsPrecedenceTest.java diff --git a/CLAUDE.md b/CLAUDE.md index 64f7b52..18ad5a8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -354,6 +354,36 @@ last eighty French names, which left **0 differences over 7 734 captured strings of the diff proves nothing; the DOM does. And the DOM does not prove everything either — it never opens the save path, which is why the two defects above needed a test apiece. +## What wins between the file and the command line + +**The command line, on every setting — and for a long time that was true of nine out of +twenty-two.** `merge` was a hand-written list of `if`s, and it stopped being complete around +the fifth option added after it. Everything absent from the list was dropped without a word +the moment a `--config` sat on the line beside it: `--level coverage` measured everything, +`--jacoco-reports data` wrote its hundred and eighty files. Nothing failed — the run simply +did something other than what it had been asked, and the only way to notice was to count the +files afterwards. Found on 29 August 2026 while adding a key, not while reading the code. + +The rule is now **applied rather than enumerated**: a setting that differs from a fresh +`Config` is one the command line set, and it overrides the file. A list has to be remembered +at every new option; a comparison does not. `SettingsPrecedenceTest` walks `Config`'s fields +by reflection and fails the build on any that does not come through — which is the guard a +longer list could never be. + +**`SERVE_HOST` says where to listen, and never that one should.** The interface a machine +exposes is a property of that machine, so it belongs in the configuration; the decision to +serve is a gesture, so it stays on the command line. That split is the whole point: a +configuration file travels — into a repository, a ticket, another machine — and one that +could open a port by travelling would be unreadable safely. `--serve` remains the only thing +that puts the tool into listening, and a test holds it: exactly one `serve = true` in the +options switch. + +Past the loopback, the report becomes readable — and annotatable — by whoever reaches the +port, and it carries the argument values captured from a real application. So the default +stays `127.0.0.1`, the key is shipped commented out with its price beside it, the tool warns +at start-up when it listens wider without a secret, and the documentation carries the +deployment that does it properly: the proxy terminates TLS and the tool answers only to it. + ## Conventions - **The tool speaks English, and so does the code now.** The switch happened in stages — the diff --git a/bin/acceptance-local.sh b/bin/acceptance-local.sh index 5dddcbe..3c69c42 100755 --- a/bin/acceptance-local.sh +++ b/bin/acceptance-local.sh @@ -207,6 +207,53 @@ grep -q '"sourcesDisponibles":{"' reassemble/index.html step $? "the annotated code is back in the report" echo +# A configuration file and a command line say different things: the line wins. It used to +# win on nine settings out of twenty-two, and to lose in silence on the others. +echo "4 sexies. A --config beside the options does not swallow them" +cat > both.conf < both.log 2>&1 +step $? "a measurement with a --config AND options ends" +grep -q "no stack sampling" both.log +step $? " --level coverage was obeyed, not the file's \"full\"" +grep -q "JACOCO_REPORTS=data" both.log +step $? " and --jacoco-reports data too" +[ "$(count both)" -lt 20 ] +step $? " which shows on the disk: $(count both) files, not the file's hundreds" + +# The same, with the file found by its name instead of named on the line: it is still a +# file, and the options typed beside it were still typed. +mkdir -p implicit +( cd implicit && cat > runtime-xray.conf < implicit.log 2>&1 ) +step $? "a measurement with the implicit runtime-xray.conf ends" +grep -q "Configuration read from" implicit/implicit.log +step $? " the file was indeed found by its name" +grep -q "no stack sampling" implicit/implicit.log +step $? " and --level coverage was obeyed all the same" + +# The interface is a setting; putting the tool into listening is a gesture. A file that +# travels — into a repository, a ticket, another machine — must not be able to open a port. +java -jar "$JAR" --config both.conf --report-only > serves-not.log 2>&1 +step $? "reading with SERVE_HOST set and no --serve ends instead of listening" +! grep -q "Report served at" serves-not.log +step $? " nothing was put into listening" +echo + echo "5. The server writes the annotations beside the measurements" PORT="$(python3 -c 'import socket;s=socket.socket();s.bind(("127.0.0.1",0));print(s.getsockname()[1]);s.close()')" java -jar "$JAR" --report-only --out out \ diff --git a/docs/outil/annotations.md b/docs/outil/annotations.md index a72862b..0258742 100644 --- a/docs/outil/annotations.md +++ b/docs/outil/annotations.md @@ -159,6 +159,75 @@ diagnostic tool, not a service: the only write it accepts is a run's annotation, whose name it chooses itself, and the file serving refuses any path that would leave the served directory. +### Deploying it, so nobody has to remember the options + +Repeating `--serve-host` on every launch is how one ends up not repeating it. Two things +carry it instead — and neither of them ever *starts* a server, which is the point: + +**In the project's configuration**, because which interface a machine exposes is a property +of that machine: + +```conf +SERVE_HOST="0.0.0.0" +``` + +> ⚠️ **This key widens what is readable, and it is off by default.** Past the loopback, the +> report — with the argument values captured from a real application — becomes readable by +> whoever reaches the port, and the annotation writes become theirs too. The key sets the +> interface and nothing else: `--serve` remains the only gesture that puts the tool into +> listening, so a configuration file copied into a repository, a ticket or another machine +> cannot open a port by travelling. + +**In a service**, for a machine that serves results permanently. The secret goes through the +environment rather than the command line, where `ps` would show it: + +```ini +# /etc/systemd/system/runtime-xray.service +[Unit] +Description=Runtime X-Ray — shared report +After=network-online.target + +[Service] +User=xray +WorkingDirectory=/srv/xray +EnvironmentFile=/etc/runtime-xray.env # XRAY_SERVE_TOKEN=…, chmod 600 +ExecStart=/usr/bin/java -jar /opt/runtime-xray/runtime-xray.jar \ + --report-only --out /srv/xray/campaigns \ + --serve 8787 --serve-host 127.0.0.1 +Restart=on-failure + +[Install] +WantedBy=multi-user.target +``` + +Note the `127.0.0.1` in a unit meant to serve a whole team: **the TLS proxy is what listens +outside**, and the tool answers only to it. That way the plain HTTP never leaves the machine, +which is what the first caveat above asks for. + +```nginx +server { + listen 443 ssl; + server_name xray.internal.example.com; + ssl_certificate /etc/ssl/certs/xray.pem; + ssl_certificate_key /etc/ssl/private/xray.key; + + location / { + proxy_pass http://127.0.0.1:8787; + proxy_set_header Host $host; + # The report is a lot of small files, and some of them are large enough to matter. + proxy_buffering off; + } +} +``` + +Two things to check before opening it, and they are the ones people skip: + +- **The output directory is the perimeter.** Everything under `--out` is served, and a + campaign carries the observed application's log and its captured values. Serve a directory + that holds only what you meant to hand over. +- **`--serve-host 0.0.0.0` without a proxy is a decision**, not a shortcut. It works, the tool + warns about it at start-up, and the warning is the whole of the protection you then have. + ## Where the file is written Seen as files, **a run is a directory**. Its annotation can live in three places, and the diff --git a/docs/outil/mode-emploi.md b/docs/outil/mode-emploi.md index 6b26869..90bf727 100644 --- a/docs/outil/mode-emploi.md +++ b/docs/outil/mode-emploi.md @@ -200,7 +200,7 @@ the first go — **[Reducing the footprint on a large codebase](empreinte.md)** | `--follow [port]` | Serves **a page showing the run in progress** (default: 8788, local loopback): activity band, busy cores, output produced, tail of the log. The `progression.jsonl` file, for its part, is **always** written: `tail -f /progression.jsonl` follows the run with no browser and no open port | | `--context ["question"]` | Writes on standard output **a bounded extract of the report, ready to hand to a language model**: the facts that answer the question, their vocabulary, and at the top what was *not* measured. Sends nothing anywhere. The families picked up are **announced on standard error**; `--help` gives the table of recognised words, **in English** — French works too, undocumented — see [Having a report read by an AI](integration-ia.md) | | `--families a,b` | Names the fact families to attach **instead of deducing them from the question**. This is the path for scripts: the result no longer depends on the words used. An unknown family stops, with the list of those that exist | -| `--serve-host ` | Listening interface (default: `127.0.0.1`). `0.0.0.0` for a shared server | +| `--serve-host ` / `SERVE_HOST` | Listening interface (default: `127.0.0.1`), or one interface in particular. **Neither ever starts a server** — only `--serve` does — so a configuration file that travels cannot open a port. Past the loopback, the report and the values captured in it become readable and annotatable by whoever reaches the port: see [deploying it](annotations.md#deploying-it-so-nobody-has-to-remember-the-options) | | `--serve-token [secret]` | Guards the served report with a **shared secret**, asked once then remembered for twelve hours. With no value, a secret is drawn at random and shown. `XRAY_SERVE_TOKEN` does the same without exposing it in `ps`. Without the option, nothing is asked: to be kept for the local loopback or an already filtered network — see [what that secret is worth](annotations.md#what-that-secret-is-worth-and-what-it-is-not) | These options combine: `--report-only --serve` serves measurements already taken, without diff --git a/orchestrator/src/main/java/lab/xray/Config.java b/orchestrator/src/main/java/lab/xray/Config.java index 24723f2..4b41d08 100644 --- a/orchestrator/src/main/java/lab/xray/Config.java +++ b/orchestrator/src/main/java/lab/xray/Config.java @@ -59,6 +59,23 @@ public final class Config { */ public String archive = ""; public String runName = ""; + /** + * The interface the served report listens on — {@code 127.0.0.1} unless said otherwise. + * + *

It is here because which interface a machine exposes is a property of that + * machine, and repeating it on every launch is how one ends up not repeating it. + * What is not here, and must not be, is the decision to serve at all: this key + * says where to listen when {@code --serve} is given, and never puts anything into + * listening by itself. A configuration file travels — into a repository, into a ticket, + * onto another machine — and a file that can open a port by travelling is a file nobody + * can read safely. + * + *

Widening it beyond the loopback publishes the captured argument values of a real + * application to whoever reaches the port, and opens the annotation writes to them too. + * The tool says so at start-up when there is no shared secret; the documentation says + * what that secret is worth. + */ + public String serveHost = "127.0.0.1"; public int attachAfterSeconds = 8; public int maxSeconds = 600; public int watchCount = 10; @@ -276,6 +293,7 @@ void set(String key, String value) { case "LEVEL", "NIVEAU" -> level = value; case "COVER_INCLUDES" -> coverIncludes = value; case "JACOCO_REPORTS" -> jacocoReports = value; + case "SERVE_HOST" -> serveHost = value; case "ARCHIVE" -> archive = value; case "SAMPLE_INTERVAL_MS" -> sampleIntervalMs = parse(value, sampleIntervalMs); case "FOLLOW_PORT", "SUIVI_PORT" -> followPort = parse(value, followPort); @@ -560,12 +578,18 @@ public static void writeTemplate(Path file) throws IOException { # or "all". The files go into /exports/. #EXPORT="cpuprofile,lcov" - # Serving the report is not set here: it is a way of launching, not a property - # of the project. "--serve" serves the output directory and lets the page write - # its annotations beside the runs; "--serve-host 0.0.0.0" makes it a shared - # server, where several people annotate in parallel, which "--serve-token" - # closes with a secret (XRAY_SERVE_TOKEN so as not to expose it in "ps"). A - # secret does not belong in a file under version control. + # WHERE the report is served, when it is. "--serve" alone starts the server and + # lets the page write its annotations beside the runs; this key only decides the + # interface it listens on, and never puts anything into listening by itself — + # a configuration file travels, and one that could open a port by travelling + # would be unreadable safely. + # + # Beyond the loopback, the report becomes readable — and annotatable — by whoever + # reaches the port: it carries the captured argument values of a real + # application. Guard it with "--serve-token" (XRAY_SERVE_TOKEN so as not to + # expose the secret in "ps"), and put TLS in front, since this speaks plain HTTP. + # A secret does not belong in a file under version control. + #SERVE_HOST="0.0.0.0" # The repository to fetch the analysis components from, once. On a closed # network, name the internal mirror: it is the only setting that matters for diff --git a/orchestrator/src/main/java/lab/xray/Main.java b/orchestrator/src/main/java/lab/xray/Main.java index c9a4fc2..7974063 100644 --- a/orchestrator/src/main/java/lab/xray/Main.java +++ b/orchestrator/src/main/java/lab/xray/Main.java @@ -76,7 +76,6 @@ private static int run(String[] args) throws Exception { boolean reportOnly = false; boolean serve = false; int servePort = 8787; - String serveHost = "127.0.0.1"; String serveToken = null; boolean randomToken = false; @@ -135,7 +134,7 @@ private static int run(String[] args) throws Exception { config.followPort = Integer.parseInt(args[++i]); } } - case "--serve-host" -> serveHost = args[++i]; + case "--serve-host" -> config.serveHost = args[++i]; case "--serve-token" -> { // The secret attaches to the option, or stays silent: "--serve-token" // alone draws one at random and shows it. That is the most frequent @@ -200,7 +199,12 @@ private static int run(String[] args) throws Exception { Path def = Path.of(DEFAULT_CONFIG); if (Files.isRegularFile(def)) { System.out.println("▶ Configuration read from: " + DEFAULT_CONFIG); - config = Config.load(def); + // Merged, exactly as an explicit --config is: a file found by its name + // rather than named on the line is still a file, and the options typed + // beside it were still typed. + Config fromFile = Config.load(def); + merge(fromFile, config); + config = fromFile; } else { Config.writeTemplate(def); announceTemplate(def, ""); @@ -283,7 +287,7 @@ private static int run(String[] args) throws Exception { System.out.println(" Pass it to whoever needs access to the " + "report."); } - LocalServer.serve(outDir, serveHost, servePort, () -> { + LocalServer.serve(outDir, config.serveHost, servePort, () -> { // After a write the page is rebuilt: the annotation becomes the report's, // and not merely this browser's. Dashboard.build(outDir, sourceRoots(served), served.watchCount, served.hidden(), @@ -880,17 +884,34 @@ private static void announceTemplate(Path file, String relaunch) { System.out.println(" java -jar runtime-xray.jar " + relaunch); } - /** The command line's options win over the file. */ - private static void merge(Config base, Config overrides) { - if (!overrides.javaCommand.isBlank()) base.javaCommand = overrides.javaCommand; - if (!overrides.rootMethod.isBlank()) base.rootMethod = overrides.rootMethod; - if (!overrides.classesDir.isBlank()) base.classesDir = overrides.classesDir; - if (!overrides.hiddenPackages.isBlank()) base.hiddenPackages = overrides.hiddenPackages; - if (!overrides.sourceDirs.isBlank()) base.sourceDirs = overrides.sourceDirs; - if (!overrides.classFilter.isBlank()) base.classFilter = overrides.classFilter; - if (!overrides.runName.isBlank()) base.runName = overrides.runName; - if (!"runtime-xray-out".equals(overrides.outDir)) base.outDir = overrides.outDir; - if (!overrides.captureValues) base.captureValues = false; + /** + * The command line's options win over the file — all of them. + * + *

This used to be a hand-written list, and it covered nine settings out of + * twenty-two. Everything else given on the command line beside a {@code --config} was + * dropped without a word: {@code --level coverage} measured everything, and + * {@code --jacoco-reports data} wrote its hundred and eighty files. Nothing failed — + * the run simply did something other than what it had been asked, and the only way to + * notice was to count the files afterwards. + * + *

So the rule is applied rather than enumerated: a setting that differs from a + * fresh {@link Config} is one the command line set, and it overrides the file. + * A list has to be remembered at every new option; this does not. + */ + static void merge(Config base, Config overrides) { + Config pristine = new Config(); + for (java.lang.reflect.Field f : Config.class.getFields()) { + if (java.lang.reflect.Modifier.isStatic(f.getModifiers())) continue; + try { + Object given = f.get(overrides); + if (java.util.Objects.equals(given, f.get(pristine))) continue; + f.set(base, given); + } catch (IllegalAccessException e) { + // A public instance field of a public class: unreachable. And a setting + // silently not carried over is exactly what this method exists to stop. + throw new IllegalStateException("setting not carried over: " + f.getName(), e); + } + } } /** @@ -1295,8 +1316,16 @@ private static void usage() { --serve [port] Serve the report (default: 8787) and let the page write its annotations next to the runs, then rebuild it. Several people can annotate at once. - --serve-host Listening interface (default: 127.0.0.1). Use 0.0.0.0 - for a shared server. + --serve-host Listening interface (default: 127.0.0.1), or a single + interface such as 10.0.0.5. SERVE_HOST says the same + in the configuration file — where a machine exposes + itself is a property of that machine. Neither ever + STARTS a server: only --serve does, so a file that + travels cannot open a port. Past the loopback the + report, and the captured values in it, are readable + and annotatable by whoever reaches the port: guard it + with --serve-token and put TLS in front — this speaks + plain HTTP. The tool says so at start-up. --serve-token [s] Guard the served report with a shared secret, asked once then remembered for 12 h. With no value, a secret is drawn at random and printed. XRAY_SERVE_TOKEN does the diff --git a/orchestrator/src/test/java/lab/xray/ConfigTest.java b/orchestrator/src/test/java/lab/xray/ConfigTest.java index 61563b2..d6d275d 100644 --- a/orchestrator/src/test/java/lab/xray/ConfigTest.java +++ b/orchestrator/src/test/java/lab/xray/ConfigTest.java @@ -181,6 +181,27 @@ void jacocoReportsIsReadFromTheFile(@TempDir Path dir) throws IOException { assertEquals("data", Config.load(file).jacocoReports); } + @Test + @DisplayName("SERVE_HOST is read from the file, and its example carries the warning") + void serveHostIsReadAndItsPriceIsStated(@TempDir Path dir) throws IOException { + Path file = dir.resolve("c.conf"); + Files.writeString(file, "JAVA_CMD=\"java -jar a.jar\"\nSERVE_HOST=\"0.0.0.0\"\n", + StandardCharsets.UTF_8); + assertEquals("0.0.0.0", Config.load(file).serveHost); + assertEquals("127.0.0.1", new Config().serveHost, "the loopback stays the default"); + + Path template = dir.resolve("t.conf"); + Config.writeTemplate(template); + String text = Files.readString(template, StandardCharsets.UTF_8); + assertTrue(text.contains("#SERVE_HOST="), "the key must be shown, commented out"); + // Whoever uncomments it must read what it costs on the same screen, not in a + // paragraph somewhere else: past the loopback, the captured values of a real + // application become readable — and the annotations writable — by whoever reaches + // the port. + assertTrue(text.contains("XRAY_SERVE_TOKEN"), "and how to guard it: " + text); + assertTrue(text.contains("TLS"), "and that this speaks plain HTTP: " + text); + } + @Test @DisplayName("The level and its settings travel with the run") void levelIsRecorded() { diff --git a/orchestrator/src/test/java/lab/xray/SettingsPrecedenceTest.java b/orchestrator/src/test/java/lab/xray/SettingsPrecedenceTest.java new file mode 100644 index 0000000..6f50be8 --- /dev/null +++ b/orchestrator/src/test/java/lab/xray/SettingsPrecedenceTest.java @@ -0,0 +1,150 @@ +package lab.xray; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Field; +import java.lang.reflect.Modifier; +import java.util.ArrayList; +import java.util.List; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * What wins when the command line and the configuration file disagree. + * + *

The answer has always been "the command line", and for a long time it was true of nine + * settings out of twenty-two. The others — the observation level, what JaCoCo writes, the + * instrumented classes, the exports, the sampling interval — were dropped without a + * word the moment a {@code --config} was on the line beside them. A run asked for at + * {@code --level coverage} measured everything; one asked for at + * {@code --jacoco-reports data} wrote a hundred and eighty files instead of nine. Nothing + * failed: the tool simply did something other than what it had been told, and the only way + * to find out was to count afterwards. + * + *

The rule is now applied rather than listed — a setting that differs from a fresh + * {@link Config} is one the command line set — and this test holds the list that a list + * would have needed. + */ +class SettingsPrecedenceTest { + + /** The file's values: everything set, and all of them different from the defaults. */ + private static Config fromFile() { + Config file = new Config(); + file.javaCommand = "java -jar from-the-file.jar"; + file.level = "coverage"; + file.jacocoReports = Config.DATA; + file.coverIncludes = "file.pkg.*"; + file.exportFormats = "lcov"; + file.sampleIntervalMs = 50; + file.serveHost = "10.0.0.1"; + file.archive = Config.KEEP; + file.outDir = "from-the-file"; + return file; + } + + @Test + @DisplayName("Every setting given on the command line survives a --config beside it") + void theCommandLineWinsOnEverySetting() { + Config file = fromFile(); + Config line = new Config(); + line.level = "tree"; + line.jacocoReports = Config.MINIMAL; + line.coverIncludes = "line.pkg.*"; + line.exportFormats = "perf"; + line.sampleIntervalMs = 7; + line.serveHost = "0.0.0.0"; + line.archive = Config.REPLACE; + line.outDir = "from-the-line"; + + Main.merge(file, line); + + assertEquals("tree", file.level); + assertEquals(Config.MINIMAL, file.jacocoReports); + assertEquals("line.pkg.*", file.coverIncludes); + assertEquals("perf", file.exportFormats); + assertEquals(7, file.sampleIntervalMs); + assertEquals("0.0.0.0", file.serveHost); + assertEquals(Config.REPLACE, file.archive); + assertEquals("from-the-line", file.outDir); + // And what the line said nothing about stays what the file said. + assertEquals("java -jar from-the-file.jar", file.javaCommand); + } + + @Test + @DisplayName("A setting the command line did not touch never overwrites the file") + void anUntouchedSettingLeavesTheFileAlone() { + // This is the other half, and the reason the rule is "differs from a fresh Config" + // rather than "is not empty": a default is not an instruction. + Config file = fromFile(); + Main.merge(file, new Config()); + assertEquals("coverage", file.level); + assertEquals(Config.DATA, file.jacocoReports); + assertEquals(50, file.sampleIntervalMs); + assertEquals("10.0.0.1", file.serveHost); + assertEquals("from-the-file", file.outDir); + } + + @Test + @DisplayName("A setting added later is carried over without anyone remembering to") + void everySettingIsCoveredWithoutAList() throws Exception { + // The defect was a hand-written list that stopped being complete. The guard is not + // a longer list: it is that every field of Config, including the one added next + // week, goes through. A field left out here fails the build rather than a campaign. + List dropped = new ArrayList<>(); + for (Field f : Config.class.getFields()) { + if (Modifier.isStatic(f.getModifiers())) continue; + Config file = new Config(); + Config line = new Config(); + Object other = different(f.get(line)); + if (other == null) continue; + f.set(line, other); + Main.merge(file, line); + if (!other.equals(f.get(file))) dropped.add(f.getName()); + } + assertTrue(dropped.isEmpty(), + "these settings are lost when a --config accompanies them: " + dropped); + } + + /** Any value that is not the one given, whatever the field's type. */ + private static Object different(Object value) { + if (value instanceof String s) return s + "-changed"; + if (value instanceof Integer i) return i + 1; + if (value instanceof Boolean b) return !b; + return null; + } + + @Test + @DisplayName("SERVE_HOST says where to listen, and never that one should") + void theFileCannotOpenAPort() throws Exception { + // A configuration file travels: into a repository, a ticket, another machine. One + // that could put a port into listening by being copied would be unreadable safely. + // So the key sets an interface, and "--serve" alone decides to serve. + Config file = Config.load(java.nio.file.Files.writeString( + java.nio.file.Files.createTempDirectory("xray").resolve("x.conf"), + "SERVE_HOST=\"0.0.0.0\"\n")); + assertEquals("0.0.0.0", file.serveHost); + + String main = java.nio.file.Files.readString(java.nio.file.Path.of("") + .toAbsolutePath().getParent() + .resolve("orchestrator/src/main/java/lab/xray/Main.java").normalize(), + java.nio.charset.StandardCharsets.UTF_8); + int start = main.indexOf("switch (a) {"); + int end = main.indexOf("unknown option", start); + assertTrue(start > 0 && end > start, "the options switch must stay locatable"); + String options = main.substring(start, end); + assertTrue(options.contains("\"--serve\""), "the region really is the options switch"); + assertEquals(1, count(options, "serve = true"), + "exactly one thing puts the tool into listening, and it is --serve"); + assertFalse(main.contains("config.serveHost.isBlank()"), + "the interface never decides whether to serve"); + } + + private static int count(String haystack, String needle) { + int n = 0; + for (int i = haystack.indexOf(needle); i >= 0; i = haystack.indexOf(needle, i + 1)) n++; + return n; + } +}