Make child-process tests portable across operating systems - #12355
Make child-process tests portable across operating systems#12355AlexeyKuznetsov-DD wants to merge 1 commit into
Conversation
AlexeyKuznetsov-DD
added
comp: testing
This comment has been minimized.
This comment has been minimized.
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
AlexeyKuznetsov-DD
removed
the
tag: ai generated
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 256262607b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| @VisibleForTesting | ||
| static String[] echo(String value, boolean emulated) { | ||
| return emulated ? emulate("echo", value) : new String[] {"echo", value}; |
There was a problem hiding this comment.
When value is option-like (for example -n, -e, or GNU --help), the native echo interprets it instead of printing it, while the Windows JVM implementation prints the literal value and a newline. This makes PortableCommand.echo(value) observably OS-dependent for valid inputs; use a literal-safe POSIX command such as printf or otherwise prevent native option parsing.
Useful? React with 👍 / 👎.
| } | ||
| switch (arguments[0]) { | ||
| case "echo": | ||
| output.write((argument(arguments) + System.lineSeparator()).getBytes(UTF_8)); |
There was a problem hiding this comment.
On Windows JVMs whose default charset is not UTF-8, such as typical JDK 8 installations, this writes UTF-8 bytes while the migrated ShellCommandExecutorTest consumes them through IOUtils.readFully, which decodes with Charset.defaultCharset(). A non-ASCII value such as é therefore becomes mojibake on the emulated path even though the API accepts arbitrary strings; encode using the inherited default charset or make callers explicitly decode UTF-8.
Useful? React with 👍 / 👎.
![datadog-prod-us1-6[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
There was a problem hiding this comment.
The new echo helper changes valid values across operating systems. POSIX treats option-like values as flags, and Windows can write non-ASCII text in a charset that the migrated consumer does not use.
🤖 Datadog Autotest · Commit 2562626 · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
|
|
||
| @VisibleForTesting | ||
| static String[] echo(String value, boolean emulated) { | ||
| return emulated ? emulate("echo", value) : new String[] {"echo", value}; |
There was a problem hiding this comment.
Tests that use option-like values can produce different results on POSIX and Windows.
Assertion details
- Input: Call
PortableCommand.echo("-n")on a POSIX system. - Expected: The command prints the literal value
-nand a line separator on all operating systems. - Actual: The POSIX command is
echo -n. The native utility treats-nas an option. It prints no value and no line separator.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest · Open Bits AI session
| } | ||
| switch (arguments[0]) { | ||
| case "echo": | ||
| output.write((argument(arguments) + System.lineSeparator()).getBytes(UTF_8)); |
There was a problem hiding this comment.
Tests with non-ASCII echo values can pass on POSIX and return incorrect output on Windows.
Assertion details
- Input: Run
PortableCommand.echowith a non-ASCII value on a Windows JVM whose default charset is not UTF-8, such as Java 8 with Cp1252. - Expected:
The migrated consumer reads the same non-ASCII value that the caller supplies. - Actual:
The child JVM writes UTF-8 bytes. The migrated consumer reads them with the default charset. On affected Windows JVMs, the consumer reads incorrect characters.
Was this helpful? React 👍 or 👎
🤖 Datadog Autotest · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest · Open Bits AI session
1 participant
What Does This Do
Adds
PortableCommandso tests can runecho,cat,sleep, and a non-terminating command without OS-specific call sites. POSIX keeps native utilities; Windows uses a small child JVM.Migrates
ProcessSupervisorTestandShellCommandExecutorTestto the portable commands.Motivation
Make child-process tests OS-independent without relying on Windows shell builtins or changing existing POSIX behavior.
Additional Notes
Validated the helper on the default JVM and Java 8, plus both migrated test classes: 43 focused test executions passed.
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issueJira ticket: N/A