MockPin 'breaks' the Pin interface?

[lurch]

According to the "Abstract Pin" documentation

Warning: Descendents must ensure that pin instances representing the same physical hardware are identical, right down to object identity.

and MockPin is a descendant of Pin. I realise the MockPin doesn't refer to actual hardware, but it might be nice (for the sake of the unit-tests) if

from gpiozero.pins.mock import MockPin
pin1 = MockPin(3)
pin2 = MockPin(3)
print(pin1 == pin2)

printed True, as it does for all the other Pin subclasses (with the current MockPin implementation it prints False).
In turn, this would allow us to have unit-tests for checking that e.g.

from gpiozero.pins.mock import MockPin
from gpiozero import LED
led1 = LED(MockPin(3))
pin2 = LED(MockPin(3))

fails with GPIOPinInUse as all the other Pin subclasses do.

I tried having a go myself at modifying the MockPin class to behave like the other Pin implementations (by changing __init__ to __new__ , adding a _PINS dict, etc.) but that just caused a whole load of problems with MockPWMPin then not working.

Feel free to close this issue, if it's irrelevant and I've misunderstood things!

[waveform80]

Yeah, unfortunately while object identity works nicely for the "real" pins, it's a pain for testing with the mock pins for a variety of reasons. The primary reason for the object identity requirement for "real" pins is that it lets us maintain state for things we can't read. So, for example, the hardware provides no way for us to determine the pull-up state of a pin - it's a "write only" property effectively. So if we want a readable property we need to remember the last state (and hope no-one's fiddled with it!). If pins are constructed and destroyed with devices we'll "lose" the pull-up state if the pin gets destroyed.

However, if we make pin objects persistent after construction and ensure that constructing pins with equivalent numbers returns exactly the same object we no longer have that issue (it's also a decent mirror of the reality: if I make two Pin(2)'s they both refer to the same hardware).

Unfortunately, in the testing scenario that makes things complex: ideally I'd like to know the state of a MockPin immediately after construction (i.e. input mode, pulled up, etc.) but if we make it like the "real pin" classes it won't be - it'll have whatever state it had at the end of the last test (and we can't guarantee an ordering of the tests) so we'll wind up having to set nearly everything on the pin after construction.

So ... I definitely have sympathy with the idea (in fact it's how I initially implemented MockPin - as a basic copy'n'paste of NativePin before stripping out the guts of it, and I even included a MockPin(2) is MockPin(2) test), but I quickly came to the conclusion it just makes testing everything else a pain.

[waveform80]

That said, when we get to writing tests for the "real" pin classes there definitely needs to be a Pin(2) is Pin(2) test in there (or something similar).

[jamesmyatt]

Do you also need to check across Pin implementations? What should the result of the following be?

pin1 = NativePin(3)
pin2 = PiGPIOPin(3)
print(pin1 == pin2)
print(pin1 is pin2)

I think that I would expect the first result to be True and the second to be False.

[lurch]

it'll have whatever state it had at the end of the last test (and we can't guarantee an ordering of the tests)

I had considered that too, but forgot to mention it during my earlier comment - we could simply use http://pytest.org/latest/fixture.html or http://pytest.org/latest/xunit_setup.html to automatically set all MockPins to a known state (and be 'unused') prior to running each test.

[lurch]

@nzbuu see #204 ;-)

[jamesmyatt]

@lurch Great 👍

[waveform80]

it'll have whatever state it had at the end of the last test (and we can't guarantee an ordering of the tests)

I had considered that too, but forgot to mention it during my earlier comment - we could simply use http://pytest.org/latest/fixture.html or http://pytest.org/latest/xunit_setup.html to automatically set all MockPins to a known state (and be 'unused') prior to running each test.

Hmm, interesting! I hadn't considered using a fixture for it ... but that would certainly centralize the setup code. I like it! No time to code it at the moment, but that's a great idea.

[lurch]

...and on the subject of MockPins and unit-tests, why is https://github.com/RPi-Distro/python-gpiozero/blob/master/tests/test_mock_pin.py#L27 an AttributeError? Shouldn't it be PinPWMUnsupported as per http://gpiozero.readthedocs.org/en/latest/api_pins.html#gpiozero.pins.Pin.frequency and https://github.com/RPi-Distro/python-gpiozero/blob/master/gpiozero/pins/mock.py#L79 ?

[waveform80]

Yeah, that one should be PinPWMUnsupported - I was probably just being lazy ("was it PinPWMUnsupported ... or PWMUnsupported or ... oh, what the hell, it had AttributeError in the parent list")!

[lurch]

So, I did a load more hacking on this, and eventually managed to get it all working :-)
https://github.com/lurch/python-gpiozero/tree/mock_tweak

But I'm not sure if it's a 'clean' solution, so I'll wait for @waveform80 's comments before pushing it as a PR.
And this now bumps us up to 60 unit-tests!

EDIT: Although the 'state' tests are obviously subject to the outcome of #212

[waveform80]

@lurch: I've had a quick look at your mock_tweak branch - looks good! Couple of things I might've done differently (up to you if you want to change them - I wouldn't mind merging it as it is):

• clear_mock_pins modifies a class attribute and only affects the class, so I probably would've made it a class method and called it something like clear_defs so teardown would become MockPin.clear_defs(), e.g.:

class MockPin(Pin):
    # ...
    @classmethod
    def clear_defs(cls):
        cls._PINS.clear()
    # ...

• I must admit it wouldn't have occurred to me to use teardown_function in that way -- I'm too used to thinking in terms of parameter fixtures. I would've gone the route of defining it like this:

@pytest.fixture(scope='function')
def pin(request):
    MockPin.clear_defs()
    return MockPin(2)

def pwm_pin(request):
    MockPWMPin.clear_defs()
    return MockPWMPin(2)

def test_some_state(pin):
    pin.state = 1
    assert pin.state == 1

def test_some_more_state(pwm_pin):
    pwm_pin.state = 0.5
    assert pwm_pin.state == 0.5

Your method of using teardown_function is much simpler (and requires significantly fewer changes to all the existing tests), but the fixture is more extensible in future. For example, if we wanted to run all tests for pins 2 and 3 (for the sake of ... having longer tests; I can't actually think of a use-case for this off the top of my head :) we could just add a parameter to the fixture and it would run all the tests for each parameter accordingly. But then this might just be my love of this feature of pytest!