Skip to content

test: add empty string and unicode edge cases to ValueTest#8219

Merged
jack-berg merged 5 commits intoopen-telemetry:mainfrom
Tusharika725:add-value-test-cases
Mar 30, 2026
Merged

test: add empty string and unicode edge cases to ValueTest#8219
jack-berg merged 5 commits intoopen-telemetry:mainfrom
Tusharika725:add-value-test-cases

Conversation

@Tusharika725
Copy link
Copy Markdown
Contributor

Description

Resolves an inline TODO in ValueTest.java to add more test cases.

Added edge cases for the valueByteAsString() method to ensure string encoding/decoding handles boundaries without data loss. Specifically added:

  • An empty string boundary.
  • A special character/Unicode boundary (testing whitespace, newlines, and a 4-byte emoji).

All local tests pass.

@Tusharika725 Tusharika725 requested a review from a team as a code owner March 27, 2026 19:32
@linux-foundation-easycla
Copy link
Copy Markdown

linux-foundation-easycla bot commented Mar 27, 2026

CLA Signed

The committers listed above are authorized under a signed CLA.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.32%. Comparing base (a88681a) to head (c38aa78).
⚠️ Report is 13 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff            @@
##               main    #8219   +/-   ##
=========================================
  Coverage     90.31%   90.32%           
  Complexity     7653     7653           
=========================================
  Files           843      843           
  Lines         23066    23080   +14     
  Branches       2310     2312    +2     
=========================================
+ Hits          20832    20846   +14     
  Misses         1516     1516           
  Partials        718      718           

☔ 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.

@jkwatson
Copy link
Copy Markdown
Contributor

It would be extra-cool if you made this into a fuzz-test that would test a wider variety of possible inputs. I think there are examples of this in the codebase somewhere. (search for "fuzz")

@Tusharika725
Copy link
Copy Markdown
Contributor Author

Tusharika725 commented Mar 28, 2026

@jkwatson Thanks for the great suggestion! I've implemented a property-based fuzz test using JQF alongside the original deterministic tests. Let me know if you need any further adjustments!

Copy link
Copy Markdown
Member

@jack-berg jack-berg left a comment

Choose a reason for hiding this comment

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

Just a nit about preferring parameterized test. Thanks for adding!

String specialBase64 = Value.of(specialStr.getBytes(StandardCharsets.UTF_8)).asString();
byte[] decodedSpecial = Base64.getDecoder().decode(specialBase64);
assertThat(new String(decodedSpecial, StandardCharsets.UTF_8)).isEqualTo(specialStr);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

When testing repeated inputs / outputs, I prefer @ParameterizedTest style, e.g.: https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk/common/src/test/java/io/opentelemetry/sdk/common/internal/GlobUtilTest.java#L17-L57

More readable and less verbose, especially when adding more test cases

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think we can probably remove this TODO

Copilot AI review requested due to automatic review settings March 30, 2026 19:09
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Adds additional coverage for Value.asString() when the underlying Value is created from UTF-8 bytes, targeting boundary/edge cases.

Changes:

  • Replaces the previous single-case valueByteAsString test with a parameterized test including an empty string and other string variants.
  • Adds a JQF/GuidedFuzzing-based fuzz test driver to validate byte<->base64<->byte round-tripping for random strings.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@jack-berg jack-berg merged commit 1685dcb into open-telemetry:main Mar 30, 2026
27 checks passed
@otelbot
Copy link
Copy Markdown
Contributor

otelbot bot commented Mar 30, 2026

Thank you for your contribution @Tusharika725! 🎉 We would like to hear from you about your experience contributing to OpenTelemetry by taking a few minutes to fill out this survey.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants