Skip to content

fix: various custom operator conformance fixes#386

Draft
leakonvalinka wants to merge 3 commits intoopen-feature:mainfrom
open-feature-forking:fix/evaluator-inconsistencies
Draft

fix: various custom operator conformance fixes#386
leakonvalinka wants to merge 3 commits intoopen-feature:mainfrom
open-feature-forking:fix/evaluator-inconsistencies

Conversation

@leakonvalinka
Copy link
Copy Markdown
Member

@leakonvalinka leakonvalinka commented Apr 24, 2026

  • support partial versions in sem_ver
  • return null instead of false for non-string inputs for starts_with and ends_with
  • rename internal parse_version to normalize_version

⚠️ wait until flagd/RPC is updated to merge and remove the added exclude-feature tags in RPC

Fixes: #379


Related:

Signed-off-by: Lea Konvalinka <lea.konvalinka@dynatrace.com>
Signed-off-by: Lea Konvalinka <lea.konvalinka@dynatrace.com>
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request updates the test-harness subproject, introduces partial version support in SemVer parsing by padding with zeros, and changes the return value of string comparisons for non-string inputs from False to None. The review feedback identifies a bug in the version padding logic that breaks versions with suffixes, notes that existing tests need updates to reflect the new return types, and suggests maintaining backward compatibility for the renamed parse_version function.

ends_with,
fractional,
parse_version,
normalize_version,
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.

medium

Renaming parse_version to normalize_version is a breaking change for consumers of this module. Given that this file is explicitly intended for backward compatibility (as noted in the header comment), it should continue to export parse_version as an alias.

Suggested change
normalize_version,
normalize_version,
normalize_version as parse_version,

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure about this one, is this necessary? I changed it locally but the pre-commit hook reverted it :D

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 it is is ok. this is an internal function, I don't think anyone relied on it. just to be a bit more carful let's add it as a change in the description.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 24, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.04%. Comparing base (6b7c78e) to head (ee5e7b6).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #386      +/-   ##
==========================================
- Coverage   96.14%   96.04%   -0.10%     
==========================================
  Files          47       33      -14     
  Lines        1737     1139     -598     
==========================================
- Hits         1670     1094     -576     
+ Misses         67       45      -22     

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

Signed-off-by: Lea Konvalinka <lea.konvalinka@dynatrace.com>
Copy link
Copy Markdown
Member

@gruebel gruebel left a comment

Choose a reason for hiding this comment

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

thanks, looks good 🍻

def test_v_prefix(self) -> None:
assert sem_ver({}, "v2.0.0", "=", "2.0.0") is True

def test_partial_version(self) -> None:
Copy link
Copy Markdown
Member

@gruebel gruebel Apr 24, 2026

Choose a reason for hiding this comment

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

can you also add the v2.1 -. 2.1.0 test?

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.

Update flagd-testbed to v3.6.2 and fix in-process evaluation edge cases

4 participants