pimoroni / mopidy-raspberry-gpio

Mopidy GPIO Control Plugin for the Raspberry Pi (Using RPi.GPIO)
Apache License 2.0
21 stars 25 forks source link

Port Mopidy-Raspberry-GPIO to Python 3 #1

Closed kingosticks closed 4 years ago

kingosticks commented 4 years ago

I'm not sure if you are aware but we have been working on porting Mopidy and extensions to Python 3 and our next major release will drop Python 2 support. It's not a huge amount of work but with support soon ending for Python 2 it will need to be done. You can find a checklist of what to do here and some examples of what we've already done with some of our extensions here and here.

Gadgetoid commented 4 years ago

I was aware of the impending Python migration- although I didn’t know how far along you were. Thanks for the step-by-step. I’ll get this done as soon as I can.

Gadgetoid commented 4 years ago

Couple of hiccups:

  1. If you're not super-vigilant with config.yml for circleci and familiar with both its and cookiecutter's syntax then it's easy to accidentally leave the {% raw %} tags in.

  2. The circleci instructions don't mention that you (in fact, a GitHub organisation administrator specifically) also have to enable (third party) "Orbs" for the organisation (for codecov)

Gadgetoid commented 4 years ago

Anyway, while trying to boost code coverage (probably erroneously since this constitutes writing tests after the fact) I ran into an issue where testing against Mopidy's API will give me a typeerror, where core.playback.volume.get() will return an object of type Mock() rather than an int.

Now I'm no Mock()ing ninja, but past experience and a dash of Google-fu suggests its possible to have Mock() return a specific value in cases like this. However, I'm at a total loss for how I might inject that value into Mopidy from my test fixtures. Is this something you've come across before?

I'm not sure if I should be attempting to inject a value here, or if it's the repsonsibility of whatever setup is being used to supplant Mopidy with a Mock() object, since it's at least reasonable to assume some more proactively written (read: less bonkers) tests might innocently do something like this and fail to pass in cases where they'd run fine against the real API.

mopidy_raspberry_gpio/frontend.py:74: TypeError
____________________________________________ test_frontend_handler_dispatch_volume_down ____________________________________________

    def test_frontend_handler_dispatch_volume_down():
        sys.modules["RPi"] = mock.Mock()
        sys.modules["RPi.GPIO"] = mock.Mock()

        frontend = frontend_lib.RaspberryGPIOFrontend(dummy_config, mock.Mock())

>       frontend.dispatch_input("volume_down")

tests/test_frontend.py:77:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
mopidy_raspberry_gpio/frontend.py:54: in dispatch_input
    getattr(self, handler_name)()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

self = <mopidy_raspberry_gpio.frontend.RaspberryGPIOFrontend object at 0xb4d68910>

    def handle_volume_down(self):
        volume = self.core.playback.volume.get()
>       volume -= 5
E       TypeError: unsupported operand type(s) for -=: 'Mock' and 'int'

mopidy_raspberry_gpio/frontend.py:80: TypeError
kingosticks commented 4 years ago

If you're not super-vigilant with config.yml for circleci and familiar with both its and cookiecutter's syntax then it's easy to accidentally leave the {% raw %} tags in.

Yes, sorry about that, I've suggested a change to the checklist which will hopefully get integrated.

The circleci instructions don't mention that you (in fact, a GitHub organisation administrator specifically) also have to enable (third party) "Orbs" for the organisation (for codecov)

It was originally written specifically for porting packages belonging to the mopidy project where that step was already done. I hit it myself when porting mopidy-tunein.

For testing, we would pass a real mopidy.core.Core core for the frontend to use rather than a Mock. But that core would be composed of "dummy" components (example, and another).

Also note that getting the volume from the playback controller is deprecated, get it from the mixer.