search

venmo / nose-detecthttp

A nose plugin that can detect tests making external http calls.
MIT License
7 stars 10 forks source link

Requests that fail are not detected #1

Open simon-weber opened 9 years ago

simon-weber commented 9 years ago

...because currently we check for recorded responses and vcr.py will not record a response for requests that fail eg during the dns lookup.

jayvdb commented 9 years ago

As the test will almost certainly fail for a different reason, is this really an issue? attempts to start http is caught. This seems more like a documentation task to me.

simon-weber commented 9 years ago

I don't entirely remember the case which led me to open this, but I remember that the test was still passing. Maybe the failure was expected? Or it wasn't under test, so not asserted on?

jayvdb commented 9 years ago

expected failures always run that risk of not verifying the desired failure, but instead verifying that some unexpected failure occurred ;-)

Was it definitely a DNS lookup which sparked this issue? Could it have been an alternative scheme which failed? e.g. irc://... or ftp://..

To play devils advocate again: If it performed a DNS lookup (successful or not), why is that within scope of this plugin whereas other types of failure within a http library (valueerrors, out of memory, disk cache, etc) would not be within scope.

If someones goal is to detect/prevent any network activity, https://github.com/ustudio/nose_connection_report/ looks like a more reliable 'approach'.

simon-weber commented 9 years ago

Was it definitely a DNS lookup which sparked this issue? Could it have been an alternative scheme which failed? e.g. irc://... or ftp://..

Yeah, it was a dns issue. I'd imagine that alternate schemes would cause a similar failure, but I'm not sure.

If someones goal is to detect/prevent any network activity, https://github.com/ustudio/nose_connection_report/ looks like a more reliable 'approach'.

Neat, I wasn't aware of that project. I think our goals are the same, but our solutions have different tradeoffs: detecthttp will give better performance at the cost of potential false negatives.