Cleanup the number of helper methods that are available for all specs #3173
Conversation
GustavoCaso
force-pushed
the
spec-helpers-cleanup
branch
2 times, most recently
from
fe31e2d to
41e83b1
Compare
GustavoCaso
changed the title
GustavoCaso
force-pushed
the
spec-helpers-cleanup
branch
from
41e83b1 to
b087037
Compare
| @@ -1,7 +1,7 @@ | |||
| require 'stringio' | |||
There was a problem hiding this comment.
Minor / probably not for this PR: This "shared" helper actually only gets used by two files in spec/datadog/core/environment/ -- I think we could perhaps move it there? (And even only include the module in those specs, rather than make it available to all others)
There was a problem hiding this comment.
That is already the case @ivoanjo
I guess the last remaining thing is move it to the spec/datadog/core/environment/ folder
There was a problem hiding this comment.
That is already the case @ivoanjo
Well, not entirely. As long as you require those specs, the helper will get required and made available for every other spec. But now that I look at them a bit closer, I think we'd need to to further changes if we wanted them to not "leak" to other specs.
So maybe just the move? ;)
There was a problem hiding this comment.
I see want you mean 😄
There was a problem hiding this comment.
I'll explore what we can do to cleanup the global helpers
| before do | ||
| allow(Datadog).to receive(:logger).and_return(double) | ||
| logger = double(Datadog::Core::Logger) | ||
| allow(logger).to receive(:debug?).and_return true | ||
| allow(logger).to receive(:debug) | ||
| allow(logger).to receive(:info) | ||
| allow(logger).to receive(:warn) | ||
| allow(logger).to receive(:error) | ||
|
|
||
| allow(Datadog).to receive(:logger).and_return(logger) | ||
| end |
There was a problem hiding this comment.
Minor: I'm curious -- why was this change needed? Was the logger being noisy and this was changed to silence it?
There was a problem hiding this comment.
The change was done to replicate the shared example in spec/datadog/tracing/buffer_spec.rb. My two cents teel me that the logger in one of the shared examples was mocking fewer methods and couldn't be used for the spec/datadog/tracing/buffer_spec.rb file, so it got copied and modified to make it pass for that case.
| @@ -22,169 +23,6 @@ | |||
| end | |||
| end | |||
|
|
|||
| RSpec.shared_examples 'thread-safe buffer' do | |||
There was a problem hiding this comment.
Weird that this was already duplicated in buffer/shared_examples...
What does this PR do?
Clean up our
spec_helperfile and reduce the number of helpers included for all specs. Also, delete duplicated shared_exmaples, and unused custom matchers.If a few specs need a helper, we should first require the spec helper directly, rather than adding as part of the global available helpers. Once we identify that a helper is widely used we can promote it to be part of all the specs. This will keep the
spec_helper.rblean, and it would be easier to understand what is that all the specs need in order to work properly.Motivation:
Additional Notes:
How to test the change?
For Datadog employees:
credentials of any kind, I've requested a review from
@DataDog/security-design-and-guidance.Unsure? Have a question? Request a review!