Avoid changing the type of socket.socket to function.

[matthewwardrop]

Thanks for providing this really useful extension to pytest.

Recently, using pytest-socket interfered with the docker package, which started subclassing socket.socket; and obviously failed since this package changes it to be a function rather than a class.

In this patch, I've replaced the socket guard with a class deriving from socket.socket, but overriding the __new__ method such that any attempt to instantiate it results in the same error as before.

[miketheman]

Hi @matthewwardrop ! This is an interesting change - how would this impact the docker package's subclassing? Would this allow the package to instantiate socket.socket?

Can you cadd a test to ensure that this new change to return a class remains tested in the future?

[matthewwardrop]

Hi @miketheman! Thanks for your review.

Currently, pytest-socket changes socket.socket to be a function instead of a class, which means that any downstream code that subclasses it will get an error along the lines of:

def guard_func():
    raise RuntimeError("whoops!")
guard_func()
---> RuntimeError("whoops!")

class MyClass(guard_func):
    pass
--> TypeError: function() argument 1 must be code, not str

With this implementation, things behave like so:

class GuardClass:
    def __new__(cls, *args, **kwargs):
        raise RuntimeError("whoops!")
GuardClass()
---> RuntimeError("whoops!")

class MyClass(GuardClass):
    pass
MyClass()
--> RuntimeError("whoops!")

which has the desired behaviour.

The only way a subclass could be instantiated is if they override the __new__ method, which is pretty rare. If they were trying to get around the pytest-socket maliciously, they could do that anyway by directly importing from socket.socket and/or resetting the module cache.

I'll add a test that checks that the type of socket.socket is a class, and that under subclassing things continue to behave as normal (although if that ever fails something is seriously wrong with Python).

[qlty-cloud-legacy]

Code Climate has analyzed commit c831c14 and detected 0 issues on this pull request.

View more on Code Climate.

[miketheman]

Thanks @matthewwardrop !

I'll include this in the next release - probably sometime later this month.

[matthewwardrop]

Thanks @miketheman !