Skip to content

Add unit tests for ValueResolver#180

Merged
yasmramos merged 5 commits intoyasmramos:developfrom
pable91:feature/issue-98
Jan 26, 2026
Merged

Add unit tests for ValueResolver#180
yasmramos merged 5 commits intoyasmramos:developfrom
pable91:feature/issue-98

Conversation

@pable91
Copy link
Copy Markdown
Contributor

@pable91 pable91 commented Jan 25, 2026

Description

This PR addresses issue #98 by adding comprehensive unit tests for the ValueResolver class.
Previously, there was insufficient test coverage for malformed expressions, environment variable handling,
and edge cases. This PR adds 27 new test cases to improve test coverage.

Test Categories Added:

  • Malformed Expression Tests (16 tests): Validates handling of incorrectly formatted placeholder expressions
  • Environment Variable Tests (2 tests): Validates resolution of environment variables and system properties
  • Edge Case Tests (9 tests): Validates various edge cases and boundary conditions

Key Test Scenarios:

  • Empty placeholders (${}, ${:}, ${:default})
  • Incomplete placeholders (unclosed braces, nested placeholders)
  • Special character handling (Unicode, escape characters, whitespace)
  • Long property names and complex default value handling

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)
  • Performance improvement
  • Refactoring (no behavior change)
  • Documentation update
  • Testing (test coverage improvement)

Testing

  • All existing tests pass
  • New tests added for changes
  • Manual testing performed

Test Execution Results:

All ValueResolverTest tests pass

./mvnw test -pl veld-runtime -Dtest=ValueResolverTest

@pable91 pable91 requested a review from yasmramos as a code owner January 25, 2026 07:47
@pable91 pable91 changed the title Feature/issue 98 Add unit tests for ValueResolver Jan 25, 2026
Copy link
Copy Markdown
Owner

@yasmramos yasmramos left a comment

Choose a reason for hiding this comment

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

Please update test assertions from Japanese to English text. Using non-ASCII characters in test strings can cause encoding issues and makes the code harder to maintain.

Change: assertEquals("値", resolver.resolve("${property.日本語}"));
To: assertEquals("value", resolver.resolve("${property.english}"));

@pable91
Copy link
Copy Markdown
Contributor Author

pable91 commented Jan 26, 2026

@yasmramos thanks

I've updated the test to keep the source ASCII-only while still verifying Unicode handling at runtime using escaped Unicode characters.

Please let me know if there’s any further feedback.

@yasmramos
Copy link
Copy Markdown
Owner

@pable91

Great, the updated approach responds well to the feedback. Thanks for implementing it so quickly. I don't have any further comments at this time. Good job!

@yasmramos yasmramos merged commit a8b5a48 into yasmramos:develop Jan 26, 2026
yasmramos added a commit that referenced this pull request Jan 26, 2026
Merge pull request #180 from pable91/feature/issue-98
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.

2 participants