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.

Sign up for GitHub

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

Jump to bottom

profiler: TestSetProfileFraction fails when run multiple times #1279

Closed
nsrip-dd opened this issue May 4, 2022 · 1 comment · Fixed by #1281
Closed

profiler: TestSetProfileFraction fails when run multiple times #1279

nsrip-dd opened this issue May 4, 2022 · 1 comment · Fixed by #1281
Assignees
@nsrip-dd

Comments

@nsrip-dd
Copy link
Contributor

nsrip-dd commented May 4, 2022

If all of the tests are run multiple times, then TestProfileFraction fails in all but the first run:

$ go test -count=4
--- FAIL: TestSetProfileFraction (0.03s)
    --- FAIL: TestSetProfileFraction/on (0.02s)
        profiler_test.go:289: 
            	Error Trace:	profiler_test.go:289
            	Error:      	Should not be: 10
            	Test:       	TestSetProfileFraction/on
--- FAIL: TestSetProfileFraction (0.04s)
    --- FAIL: TestSetProfileFraction/on (0.02s)
        profiler_test.go:289: 
            	Error Trace:	profiler_test.go:289
            	Error:      	Should not be: 10
            	Test:       	TestSetProfileFraction/on
--- FAIL: TestSetProfileFraction (0.03s)
    --- FAIL: TestSetProfileFraction/on (0.02s)
        profiler_test.go:289: 
            	Error Trace:	profiler_test.go:289
            	Error:      	Should not be: 10
            	Test:       	TestSetProfileFraction/on
FAIL
exit status 1
FAIL	gopkg.in/DataDog/dd-trace-go.v1/profiler	15.043s

This appears to have been introduced by #1262
@nsrip-dd nsrip-dd self-assigned this May 4, 2022
@nsrip-dd
Copy link
Contributor Author

nsrip-dd commented May 4, 2022

The profiler doesn't re-set the mutex profile fraction when Stop is called. (Should it?) TestAllUploaded starts the mutex profiler with the default rate. TestSetProfileFraction also uses the default rate, and if that test is run after TestAllUploaded then it will fail because it's checking that the rate after starting profiling is different than the rate before starting, which won't be the case if both values are the default rate.

I think the correct fix will be to change TestSetProfileFraction to set the mutex profile rate to 0 and then check that whether it changes from 0 (depending on the sub-test)

nsrip-dd added a commit that referenced this issue May 4, 2022
@nsrip-dd
profiler: make TestSetProfileFraction more isolated
e334ad5 
TestAllUploaded starts the mutex profiler. Currently the profiler
doesn't re-set the profile rate when Stop is called. This causes
TestSetProfileFraction/on to fail if it's run after TestAllUploaded
since the test checks that the old rate is different than the new rate,
but in both cases the rates are the default rate.

This commit changes the test to start by setting the rate to 0 and then
checking that it's either the default value or still 0 after starting
the profiler. This makes the test more isolated and not influenced by
what other tests do to the mutex profile fraction.

Fixes #1279
@nsrip-dd nsrip-dd mentioned this issue May 4, 2022
Merged
felixge added a commit that referenced this issue May 5, 2022
@nsrip-dd @felixge
profiler: make TestSetProfileFraction more isolated (#1281)
1c53df5 
TestAllUploaded starts the mutex profiler. Currently the profiler
doesn't re-set the profile rate when Stop is called. This causes
TestSetProfileFraction/on to fail if it's run after TestAllUploaded
since the test checks that the old rate is different than the new rate,
but in both cases the rates are the default rate.

This commit changes the test to start by setting the rate to 0 and then
checking that it's either the default value or still 0 after starting
the profiler. This makes the test more isolated and not influenced by
what other tests do to the mutex profile fraction.

Fixes #1279

Co-authored-by: Felix Geisendörfer <felix@datadoghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nsrip-dd nsrip-dd

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

profiler: make TestSetProfileFraction more isolated DataDog/dd-trace-go
1 participant
@nsrip-dd