search

lispci / fiveam

Common Lisp regression testing framework
BSD 3-Clause "New" or "Revised" License
185 stars 31 forks source link

Add WARNS check. #81

Open rpgoldman opened 3 years ago

rpgoldman commented 3 years ago

The SIGNALS check can correctly detect whether a block of code signals a warning, but it also aborts the code block's execution. That's appropriate for checking for an error, but not for a warning. This new check confirms that a warning of a particular class is raised, a la SIGNALS, but also allows the block of code in its scope to complete, which may be more natural for some cases.

sionescu commented 1 year ago

Good idea, but it makes more sense to modify SIGNALS and instead have it abort only when the condition is a subtype of CL:ERROR.

rpgoldman commented 1 year ago

If you would like such a mod, I could do it. I didn't do it before because changing SIGNALS would not be backward-compatible -- it could break users' previously existing tests -- but WARNS would not because it's wholly new.

Let me know what you would prefer, and I will do one of the other.

sionescu commented 1 year ago

Yes, I prefer to modify SIGNALS. It could break existing tests, but I think this is a fairly edge case that is not worth preserving.

rpgoldman commented 1 year ago

I have a new version of this, but it turned into a kludge, because it required some DWIMing.

The reason that the previously-existing signals macro does not return the value of the signaling expression is that that value may not be well-defined, especially when the condition signaled is a subclass of error.

So my DWIMed-up version of signals has the following 2 special cases:

  1. If the condition signaled is an error, then we simply return nil from the test-expression, as the original version of signals did.
  2. If the condition is a warning, we muffle that warning and continue.

In all cases except error, signals returns all values returned by evaluating its body.

Honestly, I think what would have been best would be either to pull the error special case out of signals and use signals only for conditions that continue, keep the original proposal and just add warns, or add an argument to signals to indicate whether you want the value of the body form (this last seems awfully fussy).

I have pushed the latest, kludgy version to this PR so you can look it over and make a choice.