StyraInc / regal

Regal is a linter and language server for Rego, bringing your policy development experience to the next level!
https://docs.styra.com/regal
Apache License 2.0
253 stars 34 forks source link

lsp: Address test concurrency issues #1112

Closed charlieegan3 closed 2 weeks ago

charlieegan3 commented 2 weeks ago

This PR is intended to address two issues with the test suite that happen when running tests concurrently:

TestLanguageServerSingleFile fails sometimes as the server logs an error when the context has been cancelled, and it's a panic when logging to t, after the test is over.

https://github.com/StyraInc/regal/actions/runs/10875806798/job/30174833322

TestGetInlayHintsAstTerms will fail when TestLanguageServerSingleFile is running sometimes as TestLanguageServerSingleFile updates the contents of the map while TestGetInlayHintsAstTerms is iterating over it.

https://github.com/StyraInc/regal/actions/runs/10875806798/job/30184826633

Coincidentally, both these issues happened on a recent community PR, which underlines the importance of addressing these as they're a bad contributor experience.

This PR also includes, in a second commit https://github.com/StyraInc/regal/pull/1112/commits/554f6120a575d6cdc9043284261a4eb5afdf12d3, an update to rego.QueryRegalBundle to accept a context. See commit message, but there appears to be a recurring issue here where tests timeout.

IO have updated the function an callers to pass a context to this function to allow us to better understand this issue.

charlieegan3 commented 2 weeks ago

Thanks Stephan, appreciate the review here 🙏