VRGhost / OctoPrint-PSUControl-Meross

Adds Meross Smart Plug support to OctoPrint-PSUControl
GNU Affero General Public License v3.0
4 stars 6 forks source link

Fix Meross login URL #20

Closed milmber closed 1 month ago

milmber commented 2 months ago
jeloneal commented 2 months ago

@milmber unfortunately your PR does not pass the tests. Can you take a look an fix it?

milmber commented 1 month ago

@jeloneal , looks like this test... https://github.com/VRGhost/OctoPrint-PSUControl-Meross/blob/203f87bd0336e29ee8e5f667ec6d8ece1d7b31af/test/client/test_async_client.py#L51

...fails due to the change I made here https://github.com/VRGhost/OctoPrint-PSUControl-Meross/blob/203f87bd0336e29ee8e5f667ec6d8ece1d7b31af/src/octoprint_psucontrol_meross/meross_client.py#L126

Been trying to run the tests locally (without any changes), however I get the following error:

% pip install -r requirements/develop.txt
Requirement already satisfied: pytest in ./venv/lib/python3.10/site-packages (from -r requirements/develop.txt (line 1)) (8.2.0)
Requirement already satisfied: pytest-mock in ./venv/lib/python3.10/site-packages (from -r requirements/develop.txt (line 2)) (3.14.0)
Requirement already satisfied: pytest-cov in ./venv/lib/python3.10/site-packages (from -r requirements/develop.txt (line 3)) (5.0.0)
Requirement already satisfied: pytest-asyncio in ./venv/lib/python3.10/site-packages (from -r requirements/develop.txt (line 4)) (0.23.6)
Requirement already satisfied: pyfakefs in ./venv/lib/python3.10/site-packages (from -r requirements/develop.txt (line 5)) (5.4.1)
Requirement already satisfied: asyncmock in ./venv/lib/python3.10/site-packages (from -r requirements/develop.txt (line 6)) (0.4.2)
Requirement already satisfied: iniconfig in ./venv/lib/python3.10/site-packages (from pytest->-r requirements/develop.txt (line 1)) (2.0.0)
Requirement already satisfied: packaging in ./venv/lib/python3.10/site-packages (from pytest->-r requirements/develop.txt (line 1)) (24.0)
Requirement already satisfied: pluggy<2.0,>=1.5 in ./venv/lib/python3.10/site-packages (from pytest->-r requirements/develop.txt (line 1)) (1.5.0)
Requirement already satisfied: exceptiongroup>=1.0.0rc8 in ./venv/lib/python3.10/site-packages (from pytest->-r requirements/develop.txt (line 1)) (1.2.1)
Requirement already satisfied: tomli>=1 in ./venv/lib/python3.10/site-packages (from pytest->-r requirements/develop.txt (line 1)) (2.0.1)
Requirement already satisfied: coverage>=5.2.1 in ./venv/lib/python3.10/site-packages (from coverage[toml]>=5.2.1->pytest-cov->-r requirements/develop.txt (line 3)) (7.5.1)
Requirement already satisfied: mock in ./venv/lib/python3.10/site-packages (from asyncmock->-r requirements/develop.txt (line 6)) (5.1.0)

% python test/client/test_async_client.py 
Traceback (most recent call last):
  File "/Users/milmber/IdeaProjects/OctoPrint-PSUControl-Meross/test/client/test_async_client.py", line 9, in <module>
    from octoprint_psucontrol_meross import meross_client
ModuleNotFoundError: No module named 'octoprint_psucontrol_meross'

Any tips on running the tests locally?

tsfischer commented 1 month ago

@milmber I haven't had a chance to try this, but I think you just need to change "api_url" on lines 53 and 55 of test/client/test_async_client.py to "https://api_url".

Alternatively you could probably just change it on line 55, or create a second test that only changes it on line 55, which would verify your actual fix of tacking on the https://.

jeloneal commented 1 month ago

@tsfischer tried ur proposes change but the test still fails. Any more ideas how to make the test succeed? You can propose your changes directly in @milmber fork.

tsfischer commented 1 month ago

Unfortunately I'm not familiar with the tox framework or what the intent of that test was. But since it seems to be an issue where the code is correct but the test is failing, I'd be in favor if removing the test just so this plugin can work once again. Thanks for looking into it.