Skip to content

test: Add unicode test cases - #48

Merged
liamhughes merged 1 commit into
mainfrom
liamhughes/devex-81-add-unicode-normalized-number-tests
Apr 8, 2026
Merged

test: Add unicode test cases#48
liamhughes merged 1 commit into
mainfrom
liamhughes/devex-81-add-unicode-normalized-number-tests

Conversation

@liamhughes

@liamhughes liamhughes commented Apr 8, 2026

Copy link
Copy Markdown
Contributor

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

  • Added test cases for a variety of character types.
  • Also updated a test that was asserting a default value where the evaluated value was the same as the default.

These align with these PRs for the other libraries:

Copilot AI left a comment

Copy link
Copy Markdown

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 test coverage to ensure deterministic hashing/bucketing for targeting keys containing non‑Latin and accented Unicode characters in the .NET OpenFeature provider.

Changes:

  • Added xUnit [InlineData] cases covering several Unicode scripts, emoji, and accented Latin characters for GetNormalizedNumber.
  • Adjusted one existing test’s toggle setup value (unrelated to the Unicode additions).

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

{
var featureToggles = new FeatureToggles([
new FeatureToggleEvaluation("testfeature", true, "evaluation-key", [], 100)
new FeatureToggleEvaluation("testfeature", false, "evaluation-key", [], 100)

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

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

This test’s setup change flips the toggle’s enabled value from true to false, but the test evaluates a different flag key ("anotherfeature"), so the enabled value is unused. Since this PR is focused on adding Unicode hashing cases, consider reverting this unrelated change (or add a brief note explaining why it’s needed) to keep the diff minimal and avoid confusion about the test’s intent.

Suggested change
new FeatureToggleEvaluation("testfeature", false, "evaluation-key", [], 100)
new FeatureToggleEvaluation("testfeature", true, "evaluation-key", [], 100)

Copilot uses AI. Check for mistakes.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nah. This is fine.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.


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

Comment on lines 69 to 73
public void EvaluatesToDefaultValue_IfFeatureIsNotContainedWithinSet()
{
var featureToggles = new FeatureToggles([
new FeatureToggleEvaluation("testfeature", true, "evaluation-key", [], 100)
new FeatureToggleEvaluation("testfeature", false, "evaluation-key", [], 100)
], []);

Copilot AI Apr 8, 2026

Copy link

Choose a reason for hiding this comment

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

This hunk changes the existing "EvaluatesToDefaultValue_IfFeatureIsNotContainedWithinSet" setup from an enabled to a disabled toggle. Since it’s unrelated to the stated goal of adding Unicode hashing test cases, consider reverting this change or updating the PR description to mention the test cleanup (to keep the PR focused).

Copilot uses AI. Check for mistakes.
@liamhughes
liamhughes merged commit c2830aa into main Apr 8, 2026
17 checks passed
@liamhughes
liamhughes deleted the liamhughes/devex-81-add-unicode-normalized-number-tests branch April 8, 2026 07:53
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.

3 participants