OpenCyphal / pycyphal

Python implementation of the Cyphal protocol stack.
https://pycyphal.readthedocs.io/
MIT License
119 stars 106 forks source link

Update pydsdl as requirement to 1.4 #111

Closed jxltom closed 4 years ago

jxltom commented 4 years ago

Since https://github.com/UAVCAN/pyuavcan/issues/103 is solved by https://github.com/UAVCAN/pydsdl/pull/39, pydsdl as requirement can be locked to 1.4.

Dependency nunavut also requires pydsdl 1.4, this PR solves the conflict.

pavel-kirienko commented 4 years ago

Please also freeze pytest-asyncio == 0.10 to fix the test suite. It broke because the recently released version is incompatible with PyTest v4, and we can't upgrade it because of https://github.com/UAVCAN/pyuavcan/issues/95.

A better solution would be to replace pkg_resources with importlib in Nunavut and then upgrade PyTest here to the latest version. Maybe you could do that instead?

jxltom commented 4 years ago

The tests are fixed, it looks like including all python files as tests source by python_files = *.py triggers the issue.

pavel-kirienko commented 4 years ago

This is certainly not the root cause because the filename should have nothing to do with it. Your fix silently removes unit tests that are located in files named without a leading underscore so this is undesirable.

jxltom commented 4 years ago

There are still 126 tests left, no tests are removed by this PR. It is better to name test files with prefix test_ in the further development.

My suspect is that including all files in tests might also includes setup.py or whatever which is not tests at all trigger the issue, but I have no time to dig it out

pavel-kirienko commented 4 years ago

test_*.py won't work because some tests are embedded directly in the implementation files to enhance test locality. This is an intentional design decision and I currently see no reason to change it. The underscore prefix would break the tests in the bootloader branch. The correct fix for the above problem is to fix Nunavut.

jxltom commented 4 years ago

OK, I don't know there are embbeded tests in source dir in other branch.

I will check your suggestions.

arrowcircle commented 4 years ago

This fix is also required for yukon, because it depends on nuvanut (needs 1.4.0) and pyuavcan (needs 1.2.0). Without it it's not possible to manage dependencies with pip.

jxltom commented 4 years ago

Just have some time to continue the PR. Yeah you are right. The pkg_resources is not well supported after pytest 4.6.0 as suggested here https://github.com/pytest-dev/pytest/issues/5392 and https://github.com/pytest-dev/pytest/commit/13f02af97d676bdf7143aa1834c8898fbf753d87. It is better to replace pkg_resources with importlib_resources and importlib-metadata

However, I'm not sure when I have time to do ths, so in this PR, I just pin the pytest as well as its related requirements to v4.6 temporarily.

coveralls commented 4 years ago

Pull Request Test Coverage Report for Build 802


Files with Coverage Reduction New Missed Lines %
pyuavcan/presentation/_port/_subscriber.py 1 82.46%
pyuavcan/transport/can/media/socketcan/_socketcan.py 1 82.11%
.test_dsdl_generated/uavcan/primitive/scalar/Real32_1_0.py 1 83.67%
.test_dsdl_generated/uavcan/metatransport/can/ArbitrationID_0_1.py 4 90.91%
.test_dsdl_generated/uavcan/node/GetTransportStatistics_0_1.py 4 89.76%
.test_dsdl_generated/uavcan/node/port/ID_1_0.py 4 90.91%
.test_dsdl_generated/uavcan/file/List_0_1.py 5 90.99%
.test_dsdl_generated/uavcan/file/GetInfo_0_1.py 6 92.41%
.test_dsdl_generated/uavcan/file/Modify_1_0.py 6 90.77%
.test_dsdl_generated/uavcan/metatransport/can/Manifestation_0_1.py 6 92.0%
<!-- Total: 219 -->
Totals Coverage Status
Change from base Build 782: 0.2%
Covered Lines: 18404
Relevant Lines: 20188

💛 - Coveralls