Skip to content
New issue

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

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

Already on GitHub? # to your account

feat: Allow override_flag testutil to set different checks #480

Conversation

adamantike
Copy link
Contributor

Up until now, the override_flag test util was only useful to completely activate or deactivate a flag. It didn't allow to provide more customization for all the internal checks it makes, like evaluating if the request user is authenticated or staff.

This change provides that functionality, without breaking the current API, to avoid breaking changes. Now, users can provide more granular arguments to override_flag, for a more flexible flag testing.

Closes #439.

Up until now, the `override_flag` test util was only useful to
completely activate or deactivate a flag. It didn't allow to provide
more customization for all the internal checks it makes, like evaluating
if the request user is authenticated or staff.

This change provides that functionality, without breaking the current
API, to avoid breaking changes. Now, users can provide more granular
arguments to `override_flag`, for a more flexible flag testing.

Closes jazzband#439.
Copy link
Collaborator

@clintonb clintonb left a comment

Choose a reason for hiding this comment

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

What's the use case for this? It seems the user of this functionality will ultimately either test waffle or the test itself.

For example, what does it mean to enable a flag for 25% in an automated test? Typically you want to test behavior when the flag is active vs. inactive. Setting a 25% rollout will result in flaky tests. This will be a footgun for potential users.

That user override use case is solved by instantiating the flag in the test: waffle.get_waffle_flag_model().objects.create(name='foo', everyone=None, superusers=True). However, flags are created either by migration or manually. If you create them in your test, you're really only testing your tests. If you run migrations in your test to setup the flag, your tests don't need overrides.

Flagged code doesn't care about the conditions that resulted in the flag's active state, but the state itself.

@clintonb
Copy link
Collaborator

Closing due to inactivity.

@clintonb clintonb closed this Jul 25, 2023
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Test util needed to enable flag for specific user instead of all users
2 participants