Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: issue with first variation having weight of 0 #273

Merged
merged 6 commits into from May 25, 2024

Conversation

fahad19
Copy link
Member

@fahad19 fahad19 commented Mar 7, 2024

The setup

  • Create a new feature with two variations
  • Let first variation have a weight of 0
  • Let second variation have a weight of 100

The issue

The generated datafile does not include variations in Feature.traffic.allocation.

It works though when first variation has weight of 100, and second one 0.

Workaround

Remove variations with weight of 0, or move it below another variation with a non-0 weight.

Where to fix?

Likely in datafile builder: https://github.com/featurevisor/featurevisor/blob/main/packages/core/src/builder/traffic.ts#L63

context:
country: de
expectedToBeEnabled: true
expectedVariation: treatment
Copy link
Member Author

Choose a reason for hiding this comment

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

this is failing.

Copy link
Member Author

Choose a reason for hiding this comment

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

now fixed.

// range: [0, 0],
// },

{
Copy link
Member Author

@fahad19 fahad19 Mar 7, 2024

Choose a reason for hiding this comment

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

this is failing. the allocation array is mistakenly generated as an empty array.

Copy link
Member Author

Choose a reason for hiding this comment

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

now fixed.

@fahad19 fahad19 self-assigned this Mar 7, 2024
* main: (48 commits)
  v1.22.0
  feat: show project info in CLI (#304)
  docs: deprecation (#302)
  v1.21.0
  feat: make test runner fast by default (#301)
  v1.20.0
  feat: allow evaluating in CLI with more verbosity (#300)
  docs: glossary (#291)
  v1.19.0
  feat: find duplicate segments with authors info (#299)
  v1.18.1
  fix: search features by any case in generated site (#298)
  v1.18.0
  feat: assess traffic distribution via CLI (#297)
  docs: updates
  v1.17.0
  feat: evaluate features in CLI (#295)
  v1.16.0
  feat: Added export of emitter module (#294)
  v1.15.0
  ...
@@ -22,6 +22,10 @@ export function getUpdatedAvailableRangesAfterFilling(
availableRanges: Range[],
fill: Percentage,
): Range[] {
if (fill === 0) {
Copy link
Member Author

Choose a reason for hiding this comment

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

this was the fix. this. this little if condition.

@fahad19 fahad19 marked this pull request as ready for review May 25, 2024 22:18
@fahad19 fahad19 changed the title [ISSUE] Reproduce issue with first variation having weight of 0 fix: issue with first variation having weight of 0 May 25, 2024
@fahad19 fahad19 merged commit 2853529 into main May 25, 2024
1 check passed
@fahad19 fahad19 deleted the first-variation-0-weight branch May 25, 2024 22:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant