Skip to content

Commit 8f52c60

Browse files
committed
[NO-TICKET] Fix flaky profiler object counting spec
**What does this PR do?** This PR fixes the following flaky profiler spec (seen in https://app.circleci.com/pipelines/github/DataDog/dd-trace-rb/19126/workflows/00361a70-d216-4fa9-9ce9-0f87e5f033f1/jobs/669239): ``` Failures: 1) Datadog::Profiling::Collectors::CpuAndWallTimeWorker._native_allocation_count when CpuAndWallTimeWorker has been started when allocation profiling and allocation counting is enabled returns different numbers of allocations for different threads Failure/Error: expect(after_t1 - before_t1).to be 100 expected #<Integer:201> => 100 got #<Integer:217> => 108 Compared using equal?, which compares object identity, but expected and actual are not the same object. Use `expect(actual).to eq(expected)` if you don't care about object identity in this example. # ./spec/datadog/profiling/collectors/cpu_and_wall_time_worker_spec.rb:1377:in `block (5 levels) in <top (required)>' # ./spec/spec_helper.rb:238:in `block (2 levels) in <top (required)>' # ./spec/spec_helper.rb:123:in `block (2 levels) in <top (required)>' # /usr/local/bundle/gems/webmock-3.23.0/lib/webmock/rspec.rb:39:in `block (2 levels) in <top (required)>' # /usr/local/bundle/gems/rspec-wait-0.0.9/lib/rspec/wait.rb:46:in `block (2 levels) in <top (required)>' ``` We had previously debugged the same issue on another spec in #3554 (and left a nice comment explaining what the issue is). I just propagated the same fix (disabling GC for the duration of the test) to this spec as well. **Motivation:** Zero known flaky profiler tests! **Additional Notes:** In practice what happens is that we're counting how many Ruby objects get created during the spec, but if GC gets triggered at the exact wrong time, it may trigger other Ruby code running, and thus more objects than expected being allocated. By disabling GC for this test, hopefully we're able to keep the Ruby VM from not allocating any incidental things in the background. This GC counting feature is also deprecated, so in the future we'll remove it which is even nicer :) **How to test the change?** Validate that CI is still green.
1 parent fba70fd commit 8f52c60

File tree

1 file changed

+7
-1
lines changed

1 file changed

+7
-1
lines changed

spec/datadog/profiling/collectors/cpu_and_wall_time_worker_spec.rb

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1345,6 +1345,10 @@
13451345
end
13461346

13471347
it "returns different numbers of allocations for different threads" do
1348+
# GC disabled for the same reason as "returns the exact number of allocations between two calls of the method"
1349+
# spec above.
1350+
GC.disable
1351+
13481352
# To get the exact expected number of allocations, we run this once before so that Ruby can create and cache all
13491353
# it needs to
13501354
new_object = proc { Object.new }
@@ -1374,8 +1378,10 @@
13741378
# This test checks that even though we observed 100 allocations in a background thread t1, the counters for
13751379
# the current thread were not affected by this change
13761380

1377-
expect(after_t1 - before_t1).to be 100
1381+
expect(after_t1 - before_t1).to be >= 100
13781382
expect(after_allocations - before_allocations).to be < 10
1383+
ensure
1384+
GC.enable
13791385
end
13801386

13811387
context "when allocation profiling is enabled but allocation counting is disabled" do

0 commit comments

Comments
 (0)