Overhaul testing & make tests more mandatory

[elken]

As more and more clients are added and the project grows, bugs are bound to appear. So it only makes sense to try and mitigate these by adding more tests.

The current suite is noticably brittle and difficult to manage, and while I would undertake it myself; it makes more sense to first discuss some details and come up with some standards to make future usage much smoother.

• Cask is quite buggy, that should probably be overhauled with something like eldev or even nix for the sake of CI
• Current tests are a bit flaky, so a better map of what should be tested to make removing tests easier
• Policies/documentation to enforce testing (eg no new clients without a passing test for install/initialize, any larger-than-a-typo bugfixes should add a test, etc)

And anything else I might have missed!

[yyoncho]

• Cask is quite buggy, that should probably be overhauled with something like eldev or even nix for the sake of CI

Sounds good.

• Current tests are a bit flaky, so a better map of what should be tested to make removing tests easier

Most of the test failures are related to the infra, not to the tests themself. In general, the test that checks if we bytecompile is the most useful test in this repo.

• Policies/documentation to enforce testing (eg no new clients without a passing test for install/initialize, any larger-than-a-typo bugfixes should add a test, etc)

I don't think having policies like that will be helpful - a lot of the contributors are zero/no elisp knowledge but still the PRs they are adding are useful for the community. Also, a lot of the features are not easy to test or at least they will require a lot of work. E. g. consider adding a server setting. If we go this way we will end up with a test suite that is slow and red all the time. If we have 2 active contributors per server - then fine, we can split the project in multiple repos and then let each team build their suite. But in reality, we are even struggling to cover the spec, sync up the server settings, and so on. On the positive side, I don't think that we have a lot of regressions - we used to have a lot more.

[elken]

Yeah the point on fly-by/new contributors is valid.

I think limiting it to at least just mandatory for new clients is fine then, anyone contributing an entire client will have enough knowledge of elisp and it ensures that any regressions on lesser-used servers are reported and can be fixed without it impacting users.

[jcs090218]

Cask is quite buggy, that should probably be overhauled with something like eldev or even nix for the sake of CI

I would recommend Eask, it's very similar to Cask. The previous PR is in #3465. I closed the PR due to some ert tests I am not able to resolve myself.

Most of the test failures are related to the infra, not to the tests themself. In general, the test that checks if we bytecompile is the most useful test in this repo.

Does that mean we can remove some of these tests, and re-implement the new one? 😕

[yyoncho]

Does that mean we can remove some of these tests, and re-implement the new one?

No. It is just that the code that has tests does not change very often and the tests are like 100% green all of the time and technically they don't catch any regressions.