Fix flaky HLLRedisCounter test suite
What does this MR do and why?
It fixes flaky test suite for HLLRedisCounter
It was flaky due to the known_events
entries being memoized via class variable, which caused state to leak between test cases if multiple test suites referenced HLLRedisCounter
in single process. This MR fix the issue by clearing memoized values before each test case, to prevent state leaking.
Fixes #379258 (closed)
Screenshots or screen recordings
You can emulate flaky scenario by executing two test files in the row, but first you need to comment out all but affected spec from https://gitlab.com/gitlab-org/gitlab/-/blob/master/ee/spec/lib/ee/gitlab/usage_data_counters/hll_redis_counter_spec.rb#L108
diff --git a/ee/spec/lib/ee/gitlab/usage_data_counters/hll_redis_counter_spec.rb b/ee/spec/lib/ee/gitlab/usage_data_counters/hll_redis_counter_spec.rb
index 5a9ac46891e1..b91778f0b0f4 100644
--- a/ee/spec/lib/ee/gitlab/usage_data_counters/hll_redis_counter_spec.rb
+++ b/ee/spec/lib/ee/gitlab/usage_data_counters/hll_redis_counter_spec.rb
@@ -118,102 +118,102 @@
end
end
- describe '.track_event_in_context' do
- before do
- allow(described_class).to receive(:known_events).and_return(known_events)
- end
-
- context 'with valid context' do
- where(:entity, :event_name, :context) do
- entity1 | context_event | default_context
- entity1 | context_event | ultimate_context
- entity1 | context_event | gold_context
- end
-
- with_them do
- it 'increments context event counter' do
- expect(Gitlab::Redis::HLL).to receive(:add) do |kwargs|
- expect(kwargs[:key]).to match(/^#{context}\_.*/)
- end
-
- described_class.track_event_in_context(event_name, values: entity, context: context)
- end
- end
- end
-
- context 'when sending empty context' do
- it 'is not incrementing the counter' do
- expect(Gitlab::Redis::HLL).not_to receive(:add)
-
- described_class.track_event_in_context(context_event, values: entity1, context: '')
- end
- end
- end
-
- describe '.unique_events' do
- context 'with events tracked in context' do
- before do
- allow(described_class).to receive(:known_events).and_return(known_events)
- described_class.track_event_in_context(context_event, values: [entity1, entity3], context: default_context, time: 2.days.ago)
- described_class.track_event_in_context(context_event, values: entity3, context: ultimate_context, time: 2.days.ago)
- described_class.track_event_in_context(context_event, values: entity3, context: gold_context, time: 2.days.ago)
- described_class.track_event_in_context(context_event, values: entity3, context: invalid_context, time: 2.days.ago)
- described_class.track_event_in_context(context_event, values: [entity1, entity2], context: '', time: 2.weeks.ago)
- end
-
- subject(:unique_events) { described_class.unique_events(event_names: context_event, start_date: 4.weeks.ago, end_date: Date.current, context: context) }
-
- context 'with correct arguments' do
- where(:context, :value) do
- ref(:default_context) | 2
- ref(:ultimate_context) | 1
- ref(:gold_context) | 1
- '' | 0
- end
-
- with_them do
- it { is_expected.to eq value }
- end
- end
-
- context 'with invalid context' do
- let(:context) { invalid_context }
- let(:event_names) { context_event }
-
- it 'raise error' do
- expect { unique_events }.to raise_error(Gitlab::UsageDataCounters::HLLRedisCounter::InvalidContext)
- end
- end
- end
-
- it 'does not include instrumented categories' do
- expect(described_class.unique_events_data.keys)
- .not_to include(*described_class.categories_collected_from_metrics_definitions)
- end
- end
-
- describe '.track_event' do
- before do
- allow(described_class).to receive(:known_events).and_return(known_events)
- end
-
- context 'with settings usage ping disabled' do
- before do
- stub_application_setting(usage_ping_enabled: false)
- end
-
- context 'with license usage ping enabled' do
- before do
- # License.current.customer_service_enabled? == true
- create_current_license(operational_metrics_enabled: true)
- end
-
- it 'tracks the event' do
- expect(Gitlab::Redis::HLL).to receive(:add)
-
- described_class.track_event(context_event, values: entity1, time: Date.current)
- end
- end
- end
- end
+ # describe '.track_event_in_context' do
+ # before do
+ # allow(described_class).to receive(:known_events).and_return(known_events)
+ # end
+ #
+ # context 'with valid context' do
+ # where(:entity, :event_name, :context) do
+ # entity1 | context_event | default_context
+ # entity1 | context_event | ultimate_context
+ # entity1 | context_event | gold_context
+ # end
+ #
+ # with_them do
+ # it 'increments context event counter' do
+ # expect(Gitlab::Redis::HLL).to receive(:add) do |kwargs|
+ # expect(kwargs[:key]).to match(/^#{context}\_.*/)
+ # end
+ #
+ # described_class.track_event_in_context(event_name, values: entity, context: context)
+ # end
+ # end
+ # end
+ #
+ # context 'when sending empty context' do
+ # it 'is not incrementing the counter' do
+ # expect(Gitlab::Redis::HLL).not_to receive(:add)
+ #
+ # described_class.track_event_in_context(context_event, values: entity1, context: '')
+ # end
+ # end
+ # end
+
+ # describe '.unique_events' do
+ # context 'with events tracked in context' do
+ # before do
+ # allow(described_class).to receive(:known_events).and_return(known_events)
+ # described_class.track_event_in_context(context_event, values: [entity1, entity3], context: default_context, time: 2.days.ago)
+ # described_class.track_event_in_context(context_event, values: entity3, context: ultimate_context, time: 2.days.ago)
+ # described_class.track_event_in_context(context_event, values: entity3, context: gold_context, time: 2.days.ago)
+ # described_class.track_event_in_context(context_event, values: entity3, context: invalid_context, time: 2.days.ago)
+ # described_class.track_event_in_context(context_event, values: [entity1, entity2], context: '', time: 2.weeks.ago)
+ # end
+ #
+ # subject(:unique_events) { described_class.unique_events(event_names: context_event, start_date: 4.weeks.ago, end_date: Date.current, context: context) }
+ #
+ # context 'with correct arguments' do
+ # where(:context, :value) do
+ # ref(:default_context) | 2
+ # ref(:ultimate_context) | 1
+ # ref(:gold_context) | 1
+ # '' | 0
+ # end
+ #
+ # with_them do
+ # it { is_expected.to eq value }
+ # end
+ # end
+ #
+ # context 'with invalid context' do
+ # let(:context) { invalid_context }
+ # let(:event_names) { context_event }
+ #
+ # it 'raise error' do
+ # expect { unique_events }.to raise_error(Gitlab::UsageDataCounters::HLLRedisCounter::InvalidContext)
+ # end
+ # end
+ # end
+ #
+ # it 'does not include instrumented categories' do
+ # expect(described_class.unique_events_data.keys)
+ # .not_to include(*described_class.categories_collected_from_metrics_definitions)
+ # end
+ # end
+
+ # describe '.track_event' do
+ # before do
+ # allow(described_class).to receive(:known_events).and_return(known_events)
+ # end
+ #
+ # context 'with settings usage ping disabled' do
+ # before do
+ # stub_application_setting(usage_ping_enabled: false)
+ # end
+ #
+ # context 'with license usage ping enabled' do
+ # before do
+ # # License.current.customer_service_enabled? == true
+ # create_current_license(operational_metrics_enabled: true)
+ # end
+ #
+ # it 'tracks the event' do
+ # expect(Gitlab::Redis::HLL).to receive(:add)
+ #
+ # described_class.track_event(context_event, values: entity1, time: Date.current)
+ # end
+ # end
+ # end
+ # end
end
How to set up and validate locally
Numbered steps to set up and validate the change are strongly suggested.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.