Skip to content

Add testProcess{Config} task to accept user options#581

Open
zhengyu123 wants to merge 2 commits into
mainfrom
zgu/test_cmd_opts
Open

Add testProcess{Config} task to accept user options#581
zhengyu123 wants to merge 2 commits into
mainfrom
zgu/test_cmd_opts

Conversation

@zhengyu123
Copy link
Copy Markdown
Contributor

@zhengyu123 zhengyu123 commented Jun 5, 2026 •
edited
Loading

What does this PR do?:
This PR adds a new task for testing, that accepts user options.

Motivation:
The task is created for testing new feature that avoids preloading all jmethod ids. By default, the feature is disabled and has to be enabled via fjmethodid=false argument, which is not possible with current testdebug/release tasks.

Additional Notes:

How to test the change?:
Run ./gradlew Pprofiler.options=fjmethodid=false testProcessDebug, fjmethodid=false is merged into command line that print out by tests.

For example:

[TEST] Applied profiler.options: fjmethodid=false
===> keep_jfrs: false
com.datadoghq.profiler.wallclock.SmokeWallTest > test PASSED (789ms)
com.datadoghq.profiler.wallclock.SmokeWallTest > test STARTED
[TEST] Applied profiler.options: fjmethodid=false
===> command: start,wall=1ms,fjmethodid=false,cstack=vm,attributes=tag1;tag2;tag3,jfr,file=/tmp/recordings/fp/SmokeWallTest_test14542021396455448632.jfr

For Datadog employees:

  • If this PR touches code that signs or publishes builds or packages, or handles
    credentials of any kind, I've requested a review from @DataDog/security-design-and-guidance.
  • This PR doesn't touch any of that.
  • JIRA: PROF-14940

Unsure? Have a question? Request a review!

@dd-octo-sts
Copy link
Copy Markdown
Contributor

dd-octo-sts Bot commented Jun 5, 2026

CI Test Results

Run: #27035668904 | Commit: 754464d | Duration: 12m 9s (longest job)

✅ All 32 test jobs passed

Status Overview

JDK glibc-aarch64/debug glibc-amd64/debug musl-aarch64/debug musl-amd64/debug
8 - ✅ - -
8-ibm - ✅ - -
8-j9 ✅ ✅ - -
8-librca - - ✅ ✅
8-orcl - ✅ - -
11 - ✅ - -
11-j9 ✅ ✅ - -
11-librca - - ✅ ✅
17 ✅ ✅ - -
17-graal ✅ ✅ - -
17-j9 ✅ ✅ - -
17-librca - - ✅ ✅
21 ✅ ✅ - -
21-graal ✅ ✅ - -
21-librca - - ✅ ✅
25 ✅ ✅ - -
25-graal ✅ ✅ - -
25-librca - - ✅ ✅

Legend: ✅ passed | ❌ failed | ⚪ skipped | 🚫 cancelled

Summary: Total: 32 | Passed: 32 | Failed: 0

Updated: 2026-06-05 19:45:33 UTC

@zhengyu123 zhengyu123 changed the title Zgu/test cmd opts Add testProcess{Config} task to accept user options Jun 5, 2026
@zhengyu123 zhengyu123 marked this pull request as ready for review June 5, 2026 20:24
@zhengyu123 zhengyu123 requested a review from a team as a code owner June 5, 2026 20:24
chatgpt-codex-connector[bot]
chatgpt-codex-connector Bot reviewed Jun 5, 2026
View reviewed changes
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6f5c7bae36

ℹ️ 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".

Comment thread ddprof-test/src/test/java/com/datadoghq/profiler/AbstractProfilerTest.java
Comment on lines +432 to +434
String userOptions = System.getProperty("ddprof.test.options");
if (userOptions != null && !userOptions.isEmpty()) {
profilerCommand = mergeProfilerOptions(profilerCommand, userOptions);
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot Jun 5, 2026

Choose a reason for hiding this comment

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

P2 Badge Apply profiler options to all profiler starts

When testProcessDebug is run without -Ptests, this system property is only consumed by AbstractProfilerTest; tests that call JavaProfiler.execute(...) directly still start the profiler with their hard-coded commands (for example ShutdownTest.runTest and OtelContextStorageModeTest). That means a process run requested with -Pprofiler.options=fjmethodid=false still exercises part of the suite with the default profiler options, so it is not a reliable way to validate that option across the task's full test set.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@chatgpt-codex-connector chatgpt-codex-connector[bot] chatgpt-codex-connector[bot] left review comments

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@zhengyu123