Skip to content

Conversation

@khvn26
Copy link
Member

@khvn26 khvn26 commented Sep 23, 2025

Closes #262.

Contributes to Flagsmith/flagsmith#6120, Flagsmith/flagsmith#6121.

In this PR, we:

  1. Use the new EvaluationResult schema, omitting the context key, and using a map for flags.
  2. Get rid of mapping logic used to accommodate an earlier version of engine testing data, and use new testing data more directly.

@khvn26 khvn26 requested a review from a team as a code owner September 23, 2025 16:33
@khvn26 khvn26 requested review from gagantrivedi and removed request for a team September 23, 2025 16:33
@khvn26 khvn26 changed the title feat(testing): Use EvaluationContext/EvaluationResult test data feat(tests): Use EvaluationContext/EvaluationResult test data Sep 23, 2025
@github-actions
Copy link

github-actions bot commented Sep 23, 2025

File Coverage Missing
All files 100%

Minimum allowed coverage is 100%

Generated by 🐒 cobertura-action against 1c85d1b

@codspeed-hq
Copy link

codspeed-hq bot commented Sep 23, 2025

CodSpeed Performance Report

Merging #261 will improve performances by ×20

Comparing feat/intensify-test-data (1c85d1b) with main (b4a47d7)

Summary

⚡ 1 improvement

Benchmarks breakdown

Benchmark BASE HEAD Change
test_engine_benchmark 12,408.9 µs 631.6 µs ×20

emyller
emyller previously approved these changes Sep 23, 2025
Copy link
Contributor

@emyller emyller left a comment

Choose a reason for hiding this comment

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

✂️

@emyller
Copy link
Contributor

emyller commented Sep 23, 2025

Merging #261 will degrade performances by 46.83%

This is unexpected. Perhaps caused by JSON parsing time?

@khvn26 khvn26 changed the title feat(tests): Use EvaluationContext/EvaluationResult test data feat!: Use new EvaluationResult, update test data Oct 1, 2025
@khvn26 khvn26 changed the title feat!: Use new EvaluationResult, update test data feat!: Use new EvaluationResult, updated test data Oct 1, 2025
@khvn26
Copy link
Member Author

khvn26 commented Oct 1, 2025

The base benchmark does not contain JSONPath calls — I've acknowledging the regression.

@khvn26 khvn26 requested a review from emyller October 1, 2025 16:18
@emyller
Copy link
Contributor

emyller commented Oct 1, 2025

The base benchmark does not contain JSONPath calls — I've acknowledging the regression.

FTR, I was actually pointing at the bigger size of the test data JSON file.

emyller
emyller previously approved these changes Oct 1, 2025
Copy link
Contributor

@emyller emyller left a comment

Choose a reason for hiding this comment

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

💅

emyller
emyller previously approved these changes Oct 3, 2025
@khvn26 khvn26 dismissed gagantrivedi’s stale review October 10, 2025 16:00

Comment addressed, reviewer outside working hours

@khvn26 khvn26 merged commit 8eaa386 into main Oct 10, 2025
8 checks passed
@khvn26 khvn26 deleted the feat/intensify-test-data branch October 10, 2025 16:00
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.

Use new EvaluationResult, test data format

4 participants