modules/test: check warnings/assertions

[MattSturgeon]

Extracted from #1989 to reduce its scope. Also based on earlier work from #1986.

This PR moves responsibility for assertions/warnings in tests to the test derivation, instead of printing/throwing them.

They are still printed/thrown when building a non-test nixvim build.

Additionally, two new options are added to control this behaviour. Both default to true:

• test.checkWarnings: Whether to check config.warnings in the test.
• test.checkAssertions: Whether to check config.assertions in the test.

Note

Only module warnings/assertions defined using the respective options from assertions.nix are checked.
Anything printed manually with lib.warn, builtins.trace, etc, or errors thrown with assert, throw, abort, etc will not be handled correctly by the test.

• Errors would still fail CI; the test would fail before even running
• Warnings printed manually will just print to the console, bypassing the test

In the future we can expand on this to allow "expecting" assertions, which will make our test system more flexible.

[MattSturgeon]

I wonder if it'd be worth including a test.ignoreWarnings option in this PR?

It may make the schemastore changes simpler, since we wouldn't have to avoid creating the warnings.

[khaneliman]

I wonder if it'd be worth including a test.ignoreWarnings option in this PR?

It may make the schemastore changes simpler, since we wouldn't have to avoid creating the warnings.

My first reaction is that it would be nice to have that functionality. But, I'm trying to think of how often we'd do that. Are there cases that the test might include warnings and we need to silence them so we can keep the test enabled to watch for errors? That could be nice for maintenance. I think as long as we have inline notes around why a particular test's warnings are ignored, it shouldn't cause any serious negative side effects.

[MattSturgeon]

I'm trying to think of how often we'd do that. Are there cases that the test might include warnings and we need to silence them so we can keep the test enabled to watch for errors?

The only other example is nvim-osc52, a plugin that prints a deprecation warning when enabled.

I've updated this PR to just mkForce remove the warnings.

In the future we can extend the test module with additional options, but let's not let the scope of this PR creep out of control. 😁

[MattSturgeon]

In the future we can extend the test module with additional options, but let's not let the scope of this PR creep out of control. 😁

I decided it wasn't too much work to just implement basic checkWarnigns and checkAssertions options in this PR.

[khaneliman]

LGTM, probably needs a rebase real quick though.

[MattSturgeon]

@Mergifyio queue

[mergify]

queue

✅ The pull request has been merged automatically

Details

The pull request has been merged automatically at 0641962