Migrate utils module Groovy tests to Java tests by sarahchen6 · Pull Request #11569 · DataDog/dd-trace-java

sarahchen6 · 2026-06-04T18:23:17Z

What Does This Do

Migrate Groovy tests in the utils module to Java without changing any functionality

Introduce TableTest converter for boxed values

Motivation

This is part of a broader initiative to migrate all testing in this repo to JUnit

Additional Notes

Contributor Checklist

Format the title according to the contribution guidelines
Assign the type: and (comp: or inst:) labels in addition to any other useful labels
Avoid using close, fix, or any linking keywords when referencing an issue
Use solves instead, and assign the PR milestone to the issue
Update the CODEOWNERS file on source file addition, migration, or deletion
Update public documentation with any new configuration flags or behaviors
Add your completed PR to the merge queue by commenting /merge. You can also:
- Customize the commit message associated with the merge with /merge --commit-message "..."
- Remove your PR from the merge queue with /merge -c
- Skip all merge queue checks with /merge -f --reason "reason"; please use this judiciously, as some checks do not run at the PR-level (note: the PR still needs to be mergeable, this will only skip the pre-merge build)
- Get more information in this doc

Jira ticket: [PROJ-IDENT]

sarahchen6 · 2026-06-04T18:36:01Z

+  }
+
+  @ParameterizedTest
+  @MethodSource("convertIterableMapAndBitSetArguments")


wasn't sure how to convert this to a TableTest in a simple manner since the method takes multiple types... let me know if you have recommendations!

Just pass strings and do simple parsing? Or introduce convertion function?

dd-octo-sts · 2026-06-04T18:40:01Z

🟢 Java Benchmark SLOs — All performance SLOs passed

Suite	Status
Startup	🟢 pass

SLO thresholds are defined here based on automatically generated metrics. A warning is raised when results are within 5% of the threshold.

PR vs. master results

Scenario	Candidate	master	Δ (95% CI of mean)
startup:insecure-bank:iast:Agent	13.96 s	13.98 s	[-1.0%; +0.9%] (no difference)
startup:insecure-bank:tracing:Agent	12.84 s	12.99 s	[-2.0%; -0.3%] (maybe better)
startup:petclinic:appsec:Agent	16.52 s	16.23 s	[+0.9%; +2.7%] (maybe worse)
startup:petclinic:iast:Agent	16.62 s	16.55 s	[-1.0%; +1.7%] (no difference)
startup:petclinic:profiling:Agent	16.26 s	16.47 s	[-2.8%; +0.3%] (no difference)
startup:petclinic:tracing:Agent	15.71 s	15.11 s	[-4.8%; +12.7%] (unstable)

Commit: 54ccc2aa · CI Pipeline · Benchmarking Platform UI

Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion.

sarahchen6 · 2026-06-04T19:16:12Z

+      StableConfigSource stableConfigSource =
+          new StableConfigSource(filePath.toString(), ConfigOrigin.LOCAL_STABLE_CONFIG);
+
+      // Create ConfigProvider via reflection (constructor is private)


the alternative was having this test file a stableconfig subfolder so that package-private safeToString() can be accessed 🤔

sarahchen6 · 2026-06-04T19:35:02Z

+  // spotless:on
+
+  @ParameterizedTest
+  @MethodSource("containerInfoParsedFromFileContentArguments")


This MethodSource was kept for readability -- TableTest requires inputs to be on one line

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 54ccc2aa75

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

chatgpt-codex-connector · 2026-06-04T20:58:44Z

+
+  @TableTest({
+    "configId | defaultConfigs                    ",
+    "''       | [:]                               ",


Assert parsed stable config values

For the non-empty stable-config case, this test now only checks that constructing StableConfigSource returns an object, so it would pass even if StableConfigSource silently ignored every apm_configuration_default entry. The migrated Groovy test exercised stableCfg.get(...) for each default key; keeping that assertion is important because this is the case that protects stable-config defaults from being parsed but then dropped before consumers read them.

Useful? React with 👍 / 👎.

AlexeyKuznetsov-DD

LGTM, left minor comments about code style

AlexeyKuznetsov-DD · 2026-06-04T20:51:53Z

+import java.util.Map;
+import org.junit.jupiter.api.Test;
+
+public class CapturedEnvironmentTest extends DDJavaSpecification {


Do we really need to extends DDJavaSpecification here? Or am I missing where it used?

AlexeyKuznetsov-DD · 2026-06-04T20:53:41Z

+  @Test
+  void nonAutodetectedServiceNameWithNullCommand() throws IOException, InterruptedException {
+    String serviceName = forkAndRunProperties("null");
+
+    assertNull(serviceName);
+  }
+
+  @Test
+  void nonAutodetectedServiceNameWithEmptyCommand() throws IOException, InterruptedException {
+    String serviceName = forkAndRunProperties("");
+
+    assertNull(serviceName);
+  }
+
+  @Test
+  void nonAutodetectedServiceNameWithAllBlanksCommand() throws IOException, InterruptedException {
+    String serviceName = forkAndRunProperties(" ");
+
+    assertNull(serviceName);
+  }
+
+  @Test
+  void setServiceNameBySyspropSunJavaCommandWithClass() throws IOException, InterruptedException {
+    String serviceName = forkAndRunProperties("org.example.App -Dfoo=bar arg2 arg3");
+
+    assertEquals("org.example.App", serviceName);
+  }
+
+  @Test
+  void setServiceNameBySyspropSunJavaCommandWithJar() throws IOException, InterruptedException {
+    String serviceName = forkAndRunProperties("foo/bar/example.jar -Dfoo=bar arg2 arg3");
+
+    assertEquals("example", serviceName);
+  }
+
+  @Test
+  void setServiceNameWithRealSunJavaCommandProperty() throws IOException, InterruptedException {
+    String serviceName = forkAndRunProperties(null);
+
+    assertEquals(ServiceNamePrinter.class.getName(), serviceName);
+  }
+
+  @Test
+  void useAzureSiteNameInAzure() throws IOException, InterruptedException {
+    HashMap<String, String> azureEnvVars = new HashMap<>();
+    azureEnvVars.put("DD_AZURE_APP_SERVICES", "1");
+    azureEnvVars.put("WEBSITE_SITE_NAME", "siteService");
+
+    String serviceName =
+        forkAndRunProperties("foo/bar/example.jar -Dfoo=bar arg2 arg3", azureEnvVars);
+
+    assertEquals("siteService", serviceName);
+  }
+
+  @Test
+  void dontUseSiteNameWhenNotInAzure() throws IOException, InterruptedException {
+    String serviceName =
+        forkAndRunProperties(
+            "foo/bar/example.jar -Dfoo=bar arg2 arg3",
+            Collections.singletonMap("WEBSITE_SITE_NAME", "siteService"));
+
+    assertEquals("example", serviceName);
+  }
+
+  @Test
+  void dontUseAzureSiteNameWhenNull() throws IOException, InterruptedException {
+    String serviceName =
+        forkAndRunProperties(
+            "foo/bar/example.jar -Dfoo=bar arg2 arg3",
+            Collections.singletonMap("DD_AZURE_APP_SERVICES", "true"));
+
+    assertEquals("example", serviceName);
+  }


I have feeling that all this tests can be refactored to @TableTest

AlexeyKuznetsov-DD · 2026-06-04T20:55:18Z

+  @TableTest({
+    "scenario         | key1 | key2 | value1 | value2 | origin1  | origin2 ",
+    "equal            | key  | key  | value  | value  | DEFAULT  | DEFAULT ",
+    "different key    | key  | key2 | value  | value  | ENV      | ENV     ",
+    "different value  | key  | key  | value2 | value  | JVM_PROP | JVM_PROP",
+    "different origin | key  | key  | value  | value  | ENV      | DEFAULT "
+  })
+  void supportsEqualityCheck(
+      String key1,
+      String key2,
+      Object value1,
+      Object value2,
+      ConfigOrigin origin1,
+      ConfigOrigin origin2) {


nit: may be re-group a bit: k1, v1, o1, k2, v2, o2? WDYT?

AlexeyKuznetsov-DD · 2026-06-04T20:58:11Z

+    // when
+    ConfigSetting cs1 = ConfigSetting.of(key1, value1, origin1);
+    ConfigSetting cs2 = ConfigSetting.of(key2, value2, origin2);
+
+    // then
+    if (key1.equals(key2) && value1.equals(value2) && origin1 == origin2) {
+      assertEquals(cs1.hashCode(), cs2.hashCode());
+      assertEquals(cs1, cs2);
+      assertEquals(cs2, cs1);
+      assertEquals(cs1.toString(), cs2.toString());
+    } else {
+      assertNotEquals(cs1.hashCode(), cs2.hashCode());
+      assertNotEquals(cs1, cs2);
+      assertNotEquals(cs2, cs1);
+      assertNotEquals(cs1.toString(), cs2.toString());
+    }


nit: 1. such //when & //then comments make almost no sense.
nit: 2. Probably instead of if (key1.equals(key2) && value1.equals(value2) && origin1 == origin2) {} else {}
better to introduce one more param smth. like expectedEqual and check it instead.

AlexeyKuznetsov-DD · 2026-06-04T21:00:10Z

+  }
+
+  @ParameterizedTest
+  @MethodSource("convertIterableMapAndBitSetArguments")


Just pass strings and do simple parsing? Or introduce convertion function?

AlexeyKuznetsov-DD · 2026-06-04T21:05:32Z

+    assertTrue(exception.getMessage().contains("Invalid boolean value:"));
+  }
+
+  // spotless:off


Spotless failed here? If yes, can you manually align table columns?

AlexeyKuznetsov-DD · 2026-06-04T21:06:08Z

+        expected,
+        ConfigConverter.parseMapWithOptionalMappings(mapString, "test", defaultPrefix, lowercaseKeys));
+  }
+  // spotless:on


better to do on/off in small blocks as possible.

AlexeyKuznetsov-DD · 2026-06-04T21:06:26Z

+import java.util.Map;
+import org.tabletest.junit.TableTest;
+
+public class ConfigConverterTest extends DDJavaSpecification {


Same DDJavaSpecification question... here and all similar places.

AlexeyKuznetsov-DD · 2026-06-04T21:14:15Z

+    } catch (NumberFormatException ignored) {
+    }
+    return s;
+  }


I would add empty lines between if... to improve readability...

if (source == null) return null; String s = source.toString(); if (s.isEmpty()) return null; if ("true".equals(s)) return Boolean.TRUE; if ("false".equals(s)) return Boolean.FALSE; if (s.endsWith("f")) { ...

AlexeyKuznetsov-DD · 2026-06-04T21:14:51Z

+
+public class TimeUtilsTest extends DDJavaSpecification {
+
+  // spotless:off


Spotless should work with TableTest...

sarahchen6 added 2 commits June 4, 2026 11:34

Migrate utils module Groovy to Java tests

5674e0a

Use TableTest

11e019d

sarahchen6 added comp: testing Testing tag: no release notes Changes to exclude from release notes tag: ai generated Largely based on code generated by an AI or LLM labels Jun 4, 2026

Move StableConfigMappingExceptionTest

aa40b74

sarahchen6 commented Jun 4, 2026

View reviewed changes

Convert time-utils and container-utils

c2d3578

sarahchen6 commented Jun 4, 2026

View reviewed changes

Clean up

ff836e0

sarahchen6 commented Jun 4, 2026

View reviewed changes

Use Collections.emptyMap instead of empty LinkedHashMap

54ccc2a

This comment has been minimized.

Sign in to view

sarahchen6 marked this pull request as ready for review June 4, 2026 20:55

sarahchen6 requested review from a team as code owners June 4, 2026 20:55

sarahchen6 requested a review from mhlidd June 4, 2026 20:55

chatgpt-codex-connector Bot reviewed Jun 4, 2026

View reviewed changes

AlexeyKuznetsov-DD reviewed Jun 4, 2026

View reviewed changes


		public class TimeUtilsTest extends DDJavaSpecification {

		// spotless:off

Conversation

sarahchen6 commented Jun 4, 2026

What Does This Do

Motivation

Additional Notes

Contributor Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dd-octo-sts Bot commented Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🟢 Java Benchmark SLOs — All performance SLOs passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

AlexeyKuznetsov-DD left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dd-octo-sts Bot commented Jun 4, 2026 •

edited

Loading