Fix flaky specs #2561
Fix flaky specs #2561
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2561 +/- ##
=======================================
Coverage 64.11% 64.11%
=======================================
Files 123 123
Lines 4724 4724
=======================================
Hits 3029 3029
Misses 1695 1695
|
solnic
force-pushed
the
solnic/2522-fix-flaky-specs
branch
3 times, most recently
from
65bafad to
8c292a3
Compare
Not sure what the intention was but this spec did not test any faraday error handling. What actually happened was that HTTPTransport really tried to hit the API and ended up with error. We *do not* want to hit any APIs like that in specs, this causes instability of the spec suite. That's why I added webmock to ensure we're not sending any HTTP requests in specs.
This spec was very flaky
solnic
force-pushed
the
solnic/2522-fix-flaky-specs
branch
from
8c292a3 to
0c1476b
Compare
| before do | ||
| stub_request(:post, "http://sentry.localdomain:80/sentry/api/42/envelope/"). | ||
| to_raise(Timeout::Error) | ||
| end |
There was a problem hiding this comment.
I do not know what the intention of this spec was originally since Faraday was NOT configured here and the spec was passing by an accident since there was a socket timeout raised under the hood.
I can add a faraday spec too. Just lemme know if it'd make sense.
There was a problem hiding this comment.
its a leftover from the legacy Raven SDK, ignore
There was a problem hiding this comment.
Yes we may just remove the entire test case.
There was a problem hiding this comment.
I'm gonna leave it. It does verify specific error handling - HTTPTransport defines a list of exceptions that are rescued in send_data and now this spec checks that it works as expected that the exception is wrapped in Sentry::ExternalError.
solnic
force-pushed
the
solnic/2522-fix-flaky-specs
branch
from
0c1476b to
a744a55
Compare
Our spec suite has become very unstable. I've managed to track it down to issues related to specs that set up HTTPTransport and then having request stubbing not really working. This PR adds Webmock that guarantees that we don't send HTTP requests unintentionally and keeps using our customized Net::HTTP stubbing context untouched and used the same way in specs where it is needed.
One especially problematic crash was a transport failure after spec suite run. I tracked it down to one specific spec - it lacked some configuration so I moved it to the context where that configuration is present, and the error went away, see 0c1476b.
It fixes most of the problems with flakiness, issue #2522 still lists some random failures that should be addressed so I'm keeping it open.
I think it's worth merging this in already.
Refs #2522
#skip-changelog