-
-
Notifications
You must be signed in to change notification settings - Fork 708
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
chore(deps): bump flipper, flipper-active_record and flipper-ui #12111
chore(deps): bump flipper, flipper-active_record and flipper-ui #12111
Conversation
d6ba34c
to
6c685bf
Compare
1fbedda
to
28a10b0
Compare
28a10b0
to
069f227
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great. I have a suggestion below.
config/application.rb
Outdated
@@ -253,5 +253,6 @@ module ::Reporting; end | |||
config.exceptions_app = self.routes | |||
|
|||
config.view_component.generate.sidecar = true # Always generate components in subfolders | |||
config.flipper.test_help = false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May I suggest an alternative solution?
We are doing our test setup before the whole test suite runs:
openfoodnetwork/spec/base_spec_helper.rb
Lines 99 to 103 in 5f302f3
# Reset all feature toggles to prevent leaking. | |
config.before(:suite) do | |
Flipper.features.each(&:remove) | |
OpenFoodNetwork::FeatureToggle.setup! | |
end |
If we just call the FeatureToggle.setup! before each test run then that may solve the problem, too. And then we are working with the default test config for Flipper.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @mkllnk
replace suite
with each
, is that what you're asking me to do?
config.before(:each) do
Flipper.features.each(&:remove)
OpenFoodNetwork::FeatureToggle.setup!
end
if yes, it's not working.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I still don't know what's happening here. The specs pass locally but fail on CI. Maybe it's about KnapsackPro? I don't know how they implemented the queue.
Resetting Flipper seems to happen at the right time. But maybe KnapsackPro starts a new thread which uses a new Flipper instance with a new memory adapter? It doesn't make sense to me.
069f227
to
0220933
Compare
0220933
to
1847948
Compare
Bumps [flipper](https://github.com/flippercloud/flipper), [flipper-active_record](https://github.com/flippercloud/flipper) and [flipper-ui](https://github.com/flippercloud/flipper). These dependencies needed to be updated together. Updates `flipper` from 0.26.2 to 1.2.2 - [Release notes](https://github.com/flippercloud/flipper/releases) - [Changelog](https://github.com/flippercloud/flipper/blob/main/Changelog.md) - [Commits](flippercloud/flipper@v0.26.2...v1.2.2) Updates `flipper-active_record` from 0.26.2 to 1.2.2 - [Release notes](https://github.com/flippercloud/flipper/releases) - [Changelog](https://github.com/flippercloud/flipper/blob/main/Changelog.md) - [Commits](flippercloud/flipper@v0.26.2...v1.2.2) Updates `flipper-ui` from 0.26.2 to 1.2.2 - [Release notes](https://github.com/flippercloud/flipper/releases) - [Changelog](https://github.com/flippercloud/flipper/blob/main/Changelog.md) - [Commits](flippercloud/flipper@v0.26.2...v1.2.2) --- updated-dependencies: - dependency-name: flipper dependency-type: direct:production update-type: version-update:semver-major - dependency-name: flipper-active_record dependency-type: direct:production update-type: version-update:semver-major - dependency-name: flipper-ui dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
It fails in CI and features are not seen as enabled. So we copy the important part of the test helper into our code, which works. It's probably about the point in time when the adapter is instantiated.
1847948
to
ea0558c
Compare
@abdellani, I tried using the provided test helper but I couldn't get it to work either. I think that it gets loaded at the wrong time because we initialise Flipper earlier already. We may be able to solve that by converting our custom setup code to an adapter. Maybe after the next update:
I'm not sure if it would work though. Anyway, I deactivated the test helper like you did and brought in the change to a memory store because it's faster. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice one !
Bumps flipper, flipper-active_record and flipper-ui. These dependencies needed to be updated together.
Updates
flipper
from 0.26.2 to 1.2.2Release notes
Sourced from flipper's releases.
... (truncated)
Commits
dd54868
Release 1.2.2e947540
Merge pull request #827 from flippercloud/test-help-fixesbcafbc4
Reconfigure in integration test68e2f73
TestHelp: Reconfigure once, remove all features before each test6384a80
Merge pull request #820 from flippercloud/stop-global-warming5df0121
Merge remote-tracking branch 'origin/main' into stop-global-warming52c47e4
Merge pull request #824 from flippercloud/cli-bundler-colorize12eef0e
Test colorization in specsd0c4ba7
Merge remote-tracking branch 'origin/main' into cli-bundler-colorize8ba0b5c
Try increasing process_timeout on system testUpdates
flipper-active_record
from 0.26.2 to 1.2.2Release notes
Sourced from flipper-active_record's releases.
... (truncated)
Commits
dd54868
Release 1.2.2e947540
Merge pull request #827 from flippercloud/test-help-fixesbcafbc4
Reconfigure in integration test68e2f73
TestHelp: Reconfigure once, remove all features before each test6384a80
Merge pull request #820 from flippercloud/stop-global-warming5df0121
Merge remote-tracking branch 'origin/main' into stop-global-warming52c47e4
Merge pull request #824 from flippercloud/cli-bundler-colorize12eef0e
Test colorization in specsd0c4ba7
Merge remote-tracking branch 'origin/main' into cli-bundler-colorize8ba0b5c
Try increasing process_timeout on system testUpdates
flipper-ui
from 0.26.2 to 1.2.2Release notes
Sourced from flipper-ui's releases.
... (truncated)
Commits
dd54868
Release 1.2.2e947540
Merge pull request #827 from flippercloud/test-help-fixesbcafbc4
Reconfigure in integration test68e2f73
TestHelp: Reconfigure once, remove all features before each test6384a80
Merge pull request #820 from flippercloud/stop-global-warming5df0121
Merge remote-tracking branch 'origin/main' into stop-global-warming52c47e4
Merge pull request #824 from flippercloud/cli-bundler-colorize12eef0e
Test colorization in specsd0c4ba7
Merge remote-tracking branch 'origin/main' into cli-bundler-colorize8ba0b5c
Try increasing process_timeout on system testYou can trigger a rebase of this PR by commenting
@dependabot rebase
.Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR:
@dependabot rebase
will rebase this PR@dependabot recreate
will recreate this PR, overwriting any edits that have been made to it@dependabot merge
will merge this PR after your CI passes on it@dependabot squash and merge
will squash and merge this PR after your CI passes on it@dependabot cancel merge
will cancel a previously requested merge and block automerging@dependabot reopen
will reopen this PR if it is closed@dependabot close
will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually@dependabot show <dependency name> ignore conditions
will show all of the ignore conditions of the specified dependency@dependabot ignore this major version
will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this minor version
will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)@dependabot ignore this dependency
will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)