Closed bdraco closed 1 month ago
It is quite obvious if it will break or not, because I did in fact test, and that's why I had to add the try/catch around the call, because it wasn't supported on one of the platforms I tried. (Referring only to 932)
It is quite obvious if it will break or not, because I did in fact test, and that's why I had to add the try/catch around the call, because it wasn't supported on one of the platforms I tried. (Referring only to 932)
We need to test MacOS as you mentioned in your comment that you had not tested the change on MacOS https://github.com/esphome/aioesphomeapi/pull/932#issuecomment-2318834197
I added Windows as well as its a oversight in our testing plan that we aren't running the CI on windows.
Well that was unexpected
tests/test__frame_helper.py::test_noise_frame_helper_bad_encryption PASSED [ 6%]
Windows fatal exception: access violation
Current thread 0x00001480 (most recent call first):
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\logging\__init__.py", line 392 in getMessage
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\logging\__init__.py", line 703 in format
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\logging\__init__.py", line 999 in format
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\logging\__init__.py", line 1160 in emit
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\logging\__init__.py", line 1280 in emit
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\logging\__init__.py", line 1028 in handle
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\logging\__init__.py", line 1762 in callHandlers
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\logging\__init__.py", line 1700 in handle
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\logging\__init__.py", line 1684 in _log
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\logging\__init__.py", line 1527 in debug
File "D:\a\aioesphomeapi\aioesphomeapi\aioesphomeapi\connection.py", line 714 in send_messages
File "D:\a\aioesphomeapi\aioesphomeapi\aioesphomeapi\connection.py", line 795 in send_messages_await_response_complex
File "D:\a\aioesphomeapi\aioesphomeapi\aioesphomeapi\connection.py", line 467 in _connect_hello_login
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\asyncio\events.py", line 88 in _run
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\asyncio\base_events.py", line 1986 in _run_once
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\asyncio\base_events.py", line 641 in run_forever
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\asyncio\windows_events.py", line 322 in run_forever
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\asyncio\base_events.py", line 674 in run_until_complete
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\pytest_asyncio\plugin.py", line 929 in inner
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\_pytest\python.py", line 159 in pytest_pyfunc_call
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\pluggy\_callers.py", line 103 in _multicall
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\pluggy\_manager.py", line 120 in _hookexec
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\pluggy\_hooks.py", line [51](https://github.com/esphome/aioesphomeapi/actions/runs/10621862626/job/29444832859?pr=933#step:13:52)3 in __call__
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\_pytest\python.py", line 1627 in runtest
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\pytest_asyncio\plugin.py", line 457 in runtest
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\_pytest\runner.py", line 174 in pytest_runtest_call
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\pluggy\_callers.py", line 103 in _multicall
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\pluggy\_manager.py", line 120 in _hookexec
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\pluggy\_hooks.py", line 513 in __call__
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\_pytest\runner.py", line 242 in <lambda>
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\_pytest\runner.py", line 341 in from_call
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\_pytest\runner.py", line 241 in call_and_report
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\_pytest\runner.py", line 132 in runtestprotocol
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\_pytest\runner.py", line 113 in pytest_runtest_protocol
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\pluggy\_callers.py", line 103 in _multicall
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\pluggy\_manager.py", line 120 in _hookexec
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\pluggy\_hooks.py", line 513 in __call__
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\_pytest\main.py", line 3[62](https://github.com/esphome/aioesphomeapi/actions/runs/10621862626/job/29444832859?pr=933#step:13:63) in pytest_runtestloop
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\pluggy\_callers.py", line 103 in _multicall
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\pluggy\_manager.py", line 120 in _hookexec
File "C:\hostedtoolcache\windows\Python\3.12.5\x[64](https://github.com/esphome/aioesphomeapi/actions/runs/10621862626/job/29444832859?pr=933#step:13:65)\Lib\site-packages\pluggy\_hooks.py", line 513 in __call__
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\_pytest\main.py", line 337 in _main
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\_pytest\main.py", line 283 in wrap_session
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\_pytest\main.py", line 330 in pytest_cmdline_main
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\pluggy\_callers.py", line 103 in _multicall
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\pluggy\_manager.py", line 120 in _hookexec
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\pluggy\_hooks.py", line 513 in __call__
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\_pytest\config\__init__.py", line 175 in main
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\_pytest\config\__init__.py", line 201 in console_main
File "C:\hostedtoolcache\windows\Python\3.12.5\x64\Scripts\pytest.exe\__main__.py", line 7 in <module>
File "<frozen runpy>", line 88 in _run_code
File "<frozen runpy>", line 198 in _run_module_as_main
tests/test__frame_helper.py::test_init_plaintext_with_wrong_preamble
That looks like a crash in protobuf
Never any reason not to add more test coverage.
Sadly the windows crash isn't reproducible locally so its going to be quite a bit trial and error to figure out how to work around it
[!WARNING]
Rate limit exceeded
@bdraco has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 16 minutes and 49 seconds before requesting another review.
How to resolve this issue?
After the wait time has elapsed, a review can be triggered using the `@coderabbitai review` command as a PR comment. Alternatively, push new commits to this PR. We recommend that you space out your commits to avoid hitting the rate limit.How do rate limits work?
CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our [FAQ](https://coderabbit.ai/docs/faq) for further information.Commits
Files that changed from the base of the PR and between 6cb9d057b3c12e9f4dfac0e7bef5c6c1357aca62 and a0ab8bf57510b1ad51752de11a2de31c9935d301.
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
https://github.com/esphome/aioesphomeapi/pull/932 proposed to use a new socket flag. Its likely this flag does not work on some operating systems that we support, but since we are not testing on them, its non-obvious if it will break or not.