search

openlawlibrary / pygls

A pythonic generic language server
https://pygls.readthedocs.io/en/latest/
Apache License 2.0
568 stars 103 forks source link

Flakey CI errors #367

Open tombh opened 1 year ago

tombh commented 1 year ago

Pyodide

This might just be temporary, but for the record the error is:

ERROR lsp/test_code_action.py::test_code_actions - NotImplementedError
ERROR lsp/test_completion.py::test_completion - NotImplementedError
ERROR lsp/test_diagnostics.py::test_diagnostics - NotImplementedError
ERROR lsp/test_inlay_hints.py::test_code_actions - NotImplementedError

Poetry Windows 3.12 installation

D:\a\_actions\snok\install-poetry\v1/main.sh: line 36: poetry: command not found

alcarney commented 1 year ago

I really need to dig into the install step and see if we can get a reproducible set of dependencies (in the pyodide runtime!) installed. The versions that fail are picking up a version of pytest-asyncio that tries to be helpful and close unclosed event loops - unfortunately closing an event loop is not implemented for pyodide causing the error.

The "good" runs are not picking up pytest-asyncio at all - see no plugins line here

tombh commented 1 year ago

Oh well spotted on the cause of the error there. So I assume there's at least some version of pytest-asyncio that'll get the tests to pass? Then it's just a matter of getting the Pyodide build to use pinned deps, I seem to remember you had an idea for that?

alcarney commented 1 year ago

So I assume there's at least some version of pytest-asyncio that'll get the tests to pass?

Not sure about that... it might be that it's going to be some future version... if I remember rightly the deprecation warning we see in #334 is because pytest-asyncio will not be able to close loops in future due to some upcoming API changes or something.

tombh commented 1 year ago

Oh how curious! Well pytest-asyncio randomly showing up and disappearing in itself is an issue. I don't know how that could happen, only that pytest's search path isn't deterministic? So would a fix be to simply prevent the install of pytest-asyncio in Pyodide?

alcarney commented 1 year ago

I don't know how that could happen, only that pytest's search path isn't deterministic?

It's only a hunch, but my gut tells me there's something funny going on with how the pyodide runtime installs dependencies since it uses micropip rather than pip.

So would a fix be to simply prevent the install of pytest-asyncio in Pyodide?

Probably... worst case we end up skipping some tests cases that in theory at least be run in that environment. But having something run reliably is probably an improvement overall?

tombh commented 1 year ago

That seems like a reasonable hunch, micropip isn't such a bug project it seems (not that there's anything wrong with that).

Yeah exactly, the fact that we have any tests running against Pyodide is great!

tombh commented 9 months ago

What do you think about disabling this test for now? I still don't think Github Actions have something like an allow-fail: true step? Such that we could see red for the step but green for the build.

alcarney commented 9 months ago

I think it makes sense to disable it, not sure if continue-on-error is what you are looking for?