Add test coverage for text functions

[robfrank]

Summary

• Add 49 new tests to OpenCypherTextFunctionsTest covering untested text.* functions (TextSnakeCase, TextUpperCamelCase, TextSlug, TextLevenshteinSimilarity, TextSorensenDiceSimilarity, TextRpad, TextRegexReplace, TextReplace) and edge cases for existing functions
• Create new TextStatelessFunctionsTest with 50 tests covering all SQL-style text functions (ToUpper, ToLower, Trim, LTrim, RTrim, Left, Right, Substring, Split, Replace, CharLength, Normalize, Format) that had no test coverage

Test plan

• All 133 tests pass (mvn test -Dtest="OpenCypherTextFunctionsTest,TextStatelessFunctionsTest")
• Tests cover basic functionality, null handling, edge cases, and error conditions

🤖 Generated with Claude Code

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the robustness of the database engine's text manipulation capabilities by introducing a substantial number of new unit tests. The changes ensure that both OpenCypher-specific and general SQL-style text functions are thoroughly validated across various scenarios, including basic functionality, null handling, edge cases, and error conditions, thereby improving the overall reliability and correctness of these functions.

Highlights

• Expanded OpenCypher Text Function Coverage: Added 49 new tests to OpenCypherTextFunctionsTest to cover previously untested functions such as TextSnakeCase, TextUpperCamelCase, TextSlug, TextLevenshteinSimilarity, TextSorensenDiceSimilarity, TextRpad, TextRegexReplace, and TextReplace, along with edge cases for existing functions.
• New SQL-Style Text Function Test Suite: Created a new test file, TextStatelessFunctionsTest, containing 50 tests for SQL-style text functions including ToUpper, ToLower, Trim, LTrim, RTrim, Left, Right, Substring, Split, Replace, CharLength, Normalize, and Format, which previously lacked coverage.
• Enhanced Edge Case Testing: Included additional edge case tests for various existing text functions like TextHammingDistance, TextByteCount, TextCharAt, TextCode, TextHexValue, TextFormat, TextCapitalizeAll, TextDecapitalizeAll, TextRandom, TextLpad, TextJaroWinklerDistance, and TextSplit to improve robustness.

🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console.

Changelog

• engine/src/test/java/com/arcadedb/function/text/TextStatelessFunctionsTest.java
  • Added a new test file for SQL-style stateless text functions.
• engine/src/test/java/com/arcadedb/query/opencypher/functions/OpenCypherTextFunctionsTest.java
  • Added tests for TextSnakeCase.
  • Added tests for TextUpperCamelCase.
  • Added tests for TextSlug.
  • Added tests for TextLevenshteinSimilarity.
  • Added tests for TextSorensenDiceSimilarity.
  • Added tests for TextRpad.
  • Added tests for TextRegexReplace.
  • Added tests for TextReplace.
  • Added additional edge case tests for various existing text functions.

Activity

• The pull request was generated using Claude Code.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[claude]

Code Review

Good addition of test coverage overall. The tests follow project conventions (AssertJ with assertThat, JUnit 5, final variables) and null handling is covered systematically. A few issues worth addressing.

Bug: FormatFunction tests exercise unintended behaviour

FormatFunction is a temporal formatting function — its signature is format(temporal[, pattern]) and passing a non-temporal value with a pattern throws CommandExecutionException (line 76 of FormatFunction.java).

The three tests in TextStatelessFunctionsTest pass a plain String ("hello"):

• formatFunctionNoPattern: fn.execute(new Object[]{"hello"}, null)
• formatFunctionNullPattern: fn.execute(new Object[]{"hello", null}, null)

These only pass accidentally: when no pattern is provided, the implementation falls back to args[0].toString() for any object (line 52 of FormatFunction.java). They are not testing the function's intended contract and could mislead future readers into thinking FormatFunction is a general string formatter.

Suggested fixes:

1. Replace the string inputs with actual Cypher temporal objects (CypherDate, CypherLocalTime, etc.) to test the real use-case.
2. Add a test verifying that a plain string with a pattern correctly throws CommandExecutionException.
3. Add at least one test that verifies the function actually formats a temporal value with a pattern string.

Missing edge cases

TextRegexReplace – pattern length limit (ReDoS prevention)

The implementation rejects patterns longer than 500 characters, but there is no test for this boundary:

void textRegexReplacePatternTooLong() {
    final TextRegexReplace fn = new TextRegexReplace();
    final String longPattern = "a".repeat(501);
    assertThatThrownBy(() -> fn.execute(new Object[]{"test", longPattern, "x"}, null))
        .isInstanceOf(IllegalArgumentException.class);
}

TextRpad / TextLpad – maximum length guard

Both functions throw IllegalArgumentException above 10 MB. That limit is a security-relevant guard worth testing:

void textRpadExceedsMaxLength() {
    final TextRpad fn = new TextRpad();
    assertThatThrownBy(() -> fn.execute(new Object[]{"", 10_485_761, "x"}, null))
        .isInstanceOf(IllegalArgumentException.class);
}

Minor: surprising TextSnakeCase behaviour undocumented in test

assertThat(fn.execute(new Object[]{"HELLO_WORLD"}, null)).isEqualTo("h_e_l_l_o_w_o_r_l_d");

Each uppercase letter in "HELLO_WORLD" becomes its own underscore-prefixed token because the algorithm treats every uppercase letter as a word boundary. The result is correct per the implementation, but the expected string is non-obvious. A short comment would help future maintainers.

Minor: textRpadBasic no-truncation behaviour not explicit

assertThat(fn.execute(new Object[]{"test", 2, "0"}, null)).isEqualTo("test");

The implementation returns the original string unchanged when it is already longer than the target length (no truncation). A comment stating "no truncation when string length >= target" would make the intent clear.

Good things worth noting

• Floating-point similarity comparisons correctly use isCloseTo(..., within(0.001)) rather than exact equality.
• Null handling is tested consistently for all functions.
• Unicode slug/normalize tests are appreciated.
• The @SuppressWarnings("unchecked") annotations are placed appropriately for the cast from SplitFunction.
• The test class is package-private (no public modifier) per modern JUnit 5 conventions.

[gemini-code-assist]

Code Review

This pull request adds extensive test coverage for various text functions, which is a great improvement. The new tests cover a good range of scenarios, including edge cases. I've left a few comments with suggestions to improve the tests' maintainability by reducing code duplication. I also pointed out a specific test case for text.snakeCase that validates a counter-intuitive behavior, which may indicate an underlying implementation issue.

[gemini-code-assist]

The asserted result h_e_l_l_o_w_o_r_l_d for the input HELLO_WORLD is highly counter-intuitive for a snakeCase function. The expected behavior would be to convert it to hello_world. The current test validates a behavior that is likely a bug in the TextSnakeCase implementation, where each uppercase letter is treated as a word separator even in an already underscore-separated string. I recommend changing the assertion to expect hello_world and adjusting the implementation if it fails.

Suggested change

	assertThat(fn.execute(new Object[]{"HELLO_WORLD"}, null)).isEqualTo("h_e_l_l_o_w_o_r_l_d");
	assertThat(fn.execute(new Object[]{"HELLO_WORLD"}, null)).isEqualTo("hello_world");

[robfrank]

The test documents the current implementation behavior. Changing the assertion to expect "hello_world" would make the test fail — the implementation hasn't changed. Whether this behavior is a bug is a valid question, but fixing TextSnakeCase's algorithm is out of scope for a test coverage PR. If it warrants a fix, it should be tracked separately.

[gemini-code-assist]

For better readability and to reduce code duplication, consider instantiating the stateless function objects as final fields in the test class instead of creating a new instance in each test method. Since these functions are stateless, a single instance can be reused across all tests for that function.

For example:

class TextStatelessFunctionsTest {
  private final ToUpperFunction toUpperFn = new ToUpperFunction();
  private final ToLowerFunction toLowerFn = new ToLowerFunction();
  // ... and so on

  @Test
  void toUpperBasic() {
    assertThat(toUpperFn.execute(new Object[]{"hello"}, null)).isEqualTo("HELLO");
    // ...
  }

  // ...
}

This would make the test class more concise and maintainable.

[robfrank]

The existing tests in this file (the original 34 tests before this PR) all use per-method instantiation — e.g. final TextIndexOf fn = new TextIndexOf();. Keeping the same pattern for consistency.

[gemini-code-assist]

Similar to TextStatelessFunctionsTest, many test methods in this file re-instantiate the function object. To improve code clarity and reduce repetition, you could define these stateless function objects as final fields at the class level and reuse them in the tests. This applies to all the new test sections added in this file.

[robfrank]

Same as above — per-method instantiation is the established pattern in this test file. Consistent with the existing 34 tests.

[claude]

Updated Review

(This supersedes my earlier comment, which contained inaccuracies — I had described test cases that are not present in the actual diff. Apologies for the confusion.)

Summary

The PR is a solid test-only addition. After reading the implementations against the tests, the coverage is accurate and the assertions are correct. A few observations below.

---

Bug revealed by missing test: SplitFunction NPE on null delimiter

SplitFunction (SQL-style, com.arcadedb.function.text.SplitFunction) calls:

final String delimiter = args[1].toString();   // line 44 — NPE if args[1] is null

There is a null-check for args[0] but not for args[1]. Passing a null delimiter will throw an unguarded NullPointerException rather than a meaningful error or graceful fallback.

By contrast, the OpenCypher counterpart TextSplit handles null/empty delimiter correctly (splits into individual characters).

The new tests cover splitFunctionNullReturnsNull (null string) but there is no test for a null delimiter, so this divergence goes undetected. Adding such a test would expose the implementation gap:

@Test
void splitFunctionNullDelimiterThrows() {
    final SplitFunction fn = new SplitFunction();
    // Documents that null delimiter causes NPE — implementation should be fixed
    assertThatThrownBy(() -> fn.execute(new Object[]{"abc", null}, null))
        .isInstanceOf(CommandExecutionException.class); // currently throws NPE
}

Ideally the implementation should also be fixed to match TextSplit's null handling, or at least throw CommandExecutionException instead of NPE.

---

Things done well

• FormatFunction tests: correctly use CypherDate / CypherDuration temporal objects, not plain strings. The distinction between no-pattern (toString fallback) and pattern-with-duration (throws) is both tested and accurate.
• Security boundary tests for regex pattern length (501-char → IllegalArgumentException) and lpad/rpad memory limit (10,485,761 → IllegalArgumentException) are present and correctly set one byte above the documented limits.
• TextSnakeCase: the non-obvious behaviour for "HELLO_WORLD" is documented with an inline comment — good.
• TextRpad no-truncation: the intent of fn.execute(new Object[]{"test", 2, "0"}, null) returning "test" is clarified with a comment — good.
• TextSplit null delimiter (textSplitNullDelimiter): verified against the implementation — TextSplit deliberately splits into individual characters when delimiter is null/empty, so the assertion is correct.
• Floating-point similarity assertions use isCloseTo(..., within(0.001)) — correct.
• Consistent use of final on local variables, assertThat style, package-private test class — all match project conventions.

[robfrank]

Good catch on SplitFunction's null delimiter NPE. Added a test documenting the current behavior (throws NullPointerException). The comment in the test notes the divergence from TextSplit, which handles it gracefully. Fixing the implementation is a separate concern.

[mergify]

🧪 CI Insights

Here's what we observed from your CI run for f367a95.

🟢 All jobs passed!

But CI Insights is watching 👀

[claude]

Code Review

Good increase in test coverage. The tests follow project conventions correctly: final variables, AssertJ assertions (assertThat/assertThatThrownBy), JUnit 5, and package-private class access where appropriate for unit tests. Null handling and error conditions are covered systematically. A few items worth addressing below.

---

Bug documented but not fixed: SplitFunction NPE on null delimiter

splitFunctionNullDelimiterThrowsNpe tests an actual null-pointer bug in SplitFunction:

// SplitFunction.java
final String delimiter = args[1].toString(); // NPE when args[1] is null

The test comment explicitly notes the inconsistency with TextSplit, which handles null gracefully by splitting into characters. Adding a test that documents a known bug without fixing it means the bug ships. The fix is straightforward — add a null guard to SplitFunction matching TextSplit's behaviour, then update the test to verify the fallback rather than the NPE.

---

Surprising TextSnakeCase behaviour for ALL_CAPS input

textSnakeCaseBasic asserts:

assertThat(fn.execute(new Object[]{"HELLO_WORLD"}, null)).isEqualTo("h_e_l_l_o_w_o_r_l_d");

The comment explains this is because every uppercase letter triggers an underscore. The issue is that the existing underscore separator in the input is swallowed — the function cannot round-trip through its own output. Consider also testing with already-snake-cased input:

assertThat(fn.execute(new Object[]{"hello_world"}, null)).isEqualTo("hello_world");

If that assertion fails, it reveals a more serious issue worth flagging separately.

---

Missing boundary confirmation for security limits

The security boundary tests only verify the failure side. Adding a test just below each limit confirms the boundary is exactly where expected:

// Confirm exactly 500-char pattern is accepted
assertThat(fn.execute(new Object[]{"test", "a".repeat(500), "x"}, null)).isEqualTo("test");

// Confirm exactly 10MB padding is accepted
assertThat(fn.execute(new Object[]{"", 10_485_760, "x"}, null)).hasSize(10_485_760);

Without these, a bug that set the limit to 499 or 10MB-2 would not be caught.

---

Multi-character padChar behaviour not tested

TextRpad and TextLpad accept a pad string. The implementation uses only the first character when the pad string has multiple characters. This is undocumented truncation. A test like:

assertThat(fn.execute(new Object[]{"test", 8, "ab"}, null)).isEqualTo("testaaaa"); // only 'a' used

...would document the actual behaviour and catch any future change to it.

---

TextCamelCase (lower camelCase) has no coverage

TextUpperCamelCase is tested but its sibling TextCamelCase (text.camelCase) is not. The PR description lists functions covered but text.camelCase is absent from the new tests despite being in the same directory.

---

Minor: textHexValueFromByteArray skips string input path

TextHexValue likely has two code paths — byte array and string. The test only exercises the byte array path. Adding a test with a plain string would give fuller coverage.

---

No integration-level smoke test

All new tests call function objects directly. Since these functions are also reachable through SQL/Cypher queries (e.g. RETURN text.slug('Hello World')), one or two query-level tests would guard against registration or dispatch regressions that pure unit tests would miss. Existing tests in OpenCypherTextFunctionsTest already use this pattern for older functions — consider adding at least one query-path smoke test per newly covered function group.

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
✅ -9.21%	✅ ∅

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (6dd0a35)	103053	76331	74.07%
Head commit (f367a95)	133602 (+30549)	86660 (+10329)	64.86% (-9.21%)

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#3570)	0	0	∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 65.82%. Comparing base (6dd0a35) to head (f367a95).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3570      +/-   ##
==========================================
+ Coverage   65.12%   65.82%   +0.70%     
==========================================
  Files        1506     1506              
  Lines      103053   103053              
  Branches    21366    21366              
==========================================
+ Hits        67109    67837     +728     
+ Misses      26751    26003     -748     
- Partials     9193     9213      +20     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.