Conversation
There was a problem hiding this comment.
Pull request overview
Adds additional hashing/bucketing test vectors to ensure OctopusContext.getNormalizedNumber produces stable results for non-Latin and non-ASCII targeting keys, aligning behavior with other Octopus OpenFeature provider implementations.
Changes:
- Expanded the
getNormalizedNumberMatchesExpectedValueparameterized test matrix with Unicode inputs (CJK, Arabic, Cyrillic, Hangul, emoji, accented Latin). - Minor adjustment to an existing
FlagNotFoundtest toggle setup (enabled → disabled).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var toggles = new FeatureToggles( | ||
| List.of(new FeatureToggleEvaluation("testfeature", true, "evaluation-key", Collections.emptyList(), 100)), | ||
| List.of(new FeatureToggleEvaluation("testfeature", false, "evaluation-key", Collections.emptyList(), 100)), | ||
| new byte[0] |
There was a problem hiding this comment.
This test change (setting the existing toggle from enabled to disabled) looks unrelated to the PR’s stated goal of adding Unicode hashing test cases. Since the toggle’s enabled state is not used in this assertion (the evaluated slug is different and should throw FlagNotFound regardless), consider reverting this line to keep the PR focused and avoid noisy diffs.
| Arguments.of("test-feature", "用户", 30), | ||
| Arguments.of("test-feature", "مستخدم", 19), | ||
| Arguments.of("test-feature", "ユーザー", 73), | ||
| Arguments.of("test-feature", "🎉", 54), | ||
| Arguments.of("test-feature", "café", 31), | ||
| Arguments.of("test-feature", "naïve", 28), | ||
| Arguments.of("rollout", "用户-001", 20), | ||
| Arguments.of("experiment-a", "пользователь", 81), | ||
| Arguments.of("test-feature", "사용자", 62), | ||
| Arguments.of("dark-launch", "テナント-001", 8) |
There was a problem hiding this comment.
These new cases embed non-ASCII/Unicode literals directly in the Java source. To keep tests compiling reliably on environments where the default source encoding is not UTF-8 (notably some Windows setups with JDK 11), ensure the Maven compiler is explicitly configured to use UTF-8 (e.g., via maven.compiler.encoding or maven-compiler-plugin ).
| Arguments.of("test-feature", "用户", 30), | |
| Arguments.of("test-feature", "مستخدم", 19), | |
| Arguments.of("test-feature", "ユーザー", 73), | |
| Arguments.of("test-feature", "🎉", 54), | |
| Arguments.of("test-feature", "café", 31), | |
| Arguments.of("test-feature", "naïve", 28), | |
| Arguments.of("rollout", "用户-001", 20), | |
| Arguments.of("experiment-a", "пользователь", 81), | |
| Arguments.of("test-feature", "사용자", 62), | |
| Arguments.of("dark-launch", "テナント-001", 8) | |
| Arguments.of("test-feature", "\u7528\u6237", 30), | |
| Arguments.of("test-feature", "\u0645\u0633\u062A\u062E\u062F\u0645", 19), | |
| Arguments.of("test-feature", "\u30E6\u30FC\u30B6\u30FC", 73), | |
| Arguments.of("test-feature", "\uD83C\uDF89", 54), | |
| Arguments.of("test-feature", "caf\u00E9", 31), | |
| Arguments.of("test-feature", "na\u00EFve", 28), | |
| Arguments.of("rollout", "\u7528\u6237-001", 20), | |
| Arguments.of("experiment-a", "\u043F\u043E\u043B\u044C\u0437\u043E\u0432\u0430\u0442\u0435\u043B\u044C", 81), | |
| Arguments.of("test-feature", "\uC0AC\uC6A9\uC790", 62), | |
| Arguments.of("dark-launch", "\u30C6\u30CA\u30F3\u30C8-001", 8) |
There was a problem hiding this comment.
As these are just tests, I think we can proceed and address only if it causes problems.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Background
While adding fractional evaluation to the TypeScript provider library, a gap in testing was identified: correct hashing of strings containing non-Latin alphanumeric characters.
Changes
These align with these PRs for the other libraries: