Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix liniting error in pre-commit checks #114

Closed
wants to merge 25 commits into from

Conversation

jaymunro
Copy link
Contributor

It seems this may be due to an incompatibility between black and reorder-python-imports
psf/black#4175

I have added the following to the end of the first comment in the files affected:

fmt: skip

Pre-commit is now passing tests

Addresses issue #22

@jaymunro
Copy link
Contributor Author

Is it worth it me looking into the Run tests, or was this just a matter waiting for a resolution to the pre-commit tests?

The warning given is:

tests/test_init.py::test_setup_entry_exception
  /opt/hostedtoolcache/Python/3.11.8/x64/lib/python3.11/site-packages/_pytest/python.py:183: PytestUnhandledCoroutineWarning: async def functions are not natively supported and have been skipped.
  You need to install a suitable plugin for your async framework, for example:
    - anyio
    - pytest-asyncio
    - pytest-tornasync
    - pytest-trio
    - pytest-twisted
    warnings.warn(PytestUnhandledCoroutineWarning(msg.format(nodeid)))

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

It may be this need for a further plugin or maybe it is just further test definitions needed to cover 100% of the code.

---------- coverage: platform linux, python 3.11.8-final-0 -----------
Name                                          Stmts   Miss  Cover   Missing
---------------------------------------------------------------------------
custom_components/bermuda/__init__.py           388    305    21%   63, 76, 81-100, 120-128, 148-155, 161, 167, 189-204, 216-309, 313-320, 336-359, 365-383, 387-395, 439-480, 490-494, 498-503, 506-514, 524-658, 668-669, 673, 677-679, 683-7[41](https://github.com/agittins/bermuda/actions/runs/8075052849/job/22061333681?pr=114#step:5:42), 747-807, 814-823, 830-847, 852-865, 870-871
custom_components/bermuda/binary_sensor.py       16     16     0%   2-35
custom_components/bermuda/config_flow.py         59     59     0%   2-166
custom_components/bermuda/const.py               31      0   100%
custom_components/bermuda/device_tracker.py      39     39     0%   2-68
custom_components/bermuda/entity.py              25     25     0%   2-54
custom_components/bermuda/sensor.py             109    109     0%   2-220
custom_components/bermuda/switch.py              18     18     0%   2-45
---------------------------------------------------------------------------
TOTAL                                           685    571    17%

FAIL Required test coverage of 100.0% not reached. Total coverage: 16.64%

agittins added a commit that referenced this pull request Feb 28, 2024
- refer psf/black#4175
- issue identified by jaymunro #114
- bumped pre-commit-hooks repo to v4.5.0
@agittins
Copy link
Owner

The tests issue requires writing tests for full code coverage. I'm completely without knowledge on the test framework so I suspect I'd need to spend a few hours getting my head around it and learning, and quite possibly more hours learning the special-case workarounds required on top of the general test framework itself.

So... yeah :-/

It's something that will probably become more of a priority for me later in the project, especially as it gets more complicated. But for now I'm not going to spend any time on it unless my ADHD suddenly takes an interest 😆

As for the linting....

Well done tracking down the root cause of that issue! From the discussion it looks like isort might become a preferred import-sorting in the longer term.

I have a personal issue with a tool that's meant to make code cleaner requiring spraying of directives around. Also, this seems to be due to a bug in reorder-imports maintainer's brain, and he's being a douchebag about it. Since he's betraying the singular stated goal of the tool, I'd rather switch to something else.

I've raised a pr at #115 that does this.
So sorry I won't be merging this PR, but I do appreciate the leg-work you did to hunt down the issue, thanks!

@agittins agittins closed this Feb 28, 2024
agittins added a commit that referenced this pull request Feb 28, 2024
* wip: Trying to get passive updates

- nothing functional yet, but since nothing breaks either, committing it for now.
- plan is to allow updates to be triggered by incoming advertisements from devices we are maintaining sensors for.

* chore: Move from reorder-imports to isort

- refer psf/black#4175
- issue identified by jaymunro #114
- bumped pre-commit-hooks repo to v4.5.0

* chore: update black and apply formattings

- disabled check on const line because back-and-forth linting conflicts should not take more than three hours to solve.

- added black to requirements and bumped to same version in github workflow

- gitignore node_modules/.cache for new prettier version.
@jaymunro jaymunro deleted the pre-commit-fixes branch March 8, 2024 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants