Replacing -lit-test with a test utility?

Hi,

We were discussing https://reviews.llvm.org/D50641 over lunch and an idea to replace -lit-test by an utility appeared (shoutout to Dan Liew!).

I don’t particularly like to have specific code path for testing but I see the value it provides. How about I implement a small python script or a binary that would read JSON strings separated by triple-dash from stdin and write proper LSP messages to stdout (with LSP headers prepended)?

What do you think?

Cheers,

Jan

Hi,

We were discussing https://reviews.llvm.org/D50641 over lunch and an idea to replace -lit-test by an utility appeared (shoutout to Dan Liew!).

I don’t particularly like to have specific code path for testing but I see the value it provides. How about I implement a small python script or a binary that would read JSON strings separated by triple-dash from stdin and write proper LSP messages to stdout (with LSP headers prepended)?

How would this help with the bugs in D50641? AFAICT, the non-lit code path wouldn’t catch the bugs either; it also returns None with some error message. As Alex suggested in the patch, I also think it would be more helpful if we could exit clangd with failure on malformed message (when -lit-test is set).

Hi Jan,

I agree that ‘-input-style=delimited’ and ‘-lit-test’ does look like something that shouldn’t be in clangd from the architectural standpoint, as it’s only specific to the test.

In practice, the python wrapper will probably be more complicated than what we have now and could even make some things harder, e.g. it’s very easy to debug failures now and I could imagine problems with debugging a python wrapper.

Hi,

It also doesn’t seem to me that the benefits outweigh the costs. (Arguments below). Not strongly opposed if others like it, but please make it C++ rather than python. (Happy to elaborate if that’s controversial).

The divergence in code is small and isolated, and the production code is covered by lit tests. This change to the tests seems unlikely to catch any actual bugs. It seems just as likely we’ll have bugs in the transcoder.

In addition to a more maintainable input format, -lit-test triggers a couple more behaviors that are important for testing but not particularly relevant to any particular test. One of the (IMO) antipatterns of lit tests is they accumulate flags and noise that aren’t particularly related to the current test and obscure intent. Splitting out the transcoder would make it harder to bundle our generic test behavior this way, as well as adding noise to each test for pipeline mechanics.