unisonweb / unison

A friendly programming language from the future
https://unison-lang.org
Other
5.81k stars 271 forks source link

support “known failures” in transcripts #5350

Open sellout opened 2 months ago

sellout commented 2 months ago

Is your feature request related to a problem? Please describe.

I mentioned this in https://github.com/unisonweb/unison/pull/5263#issuecomment-2269837881, so there is some ensuing discussion there.

@aryairani and I have both triaged issues, writing transcripts to see if they’re still issues or not, but then there is nothing to do with the transcript if they are still issues. The next person to triage needs to write those transcripts again and test them out.

This would prevent that work and also allow the org to be more pro-active about resolving issues as soon as they are addressed.

Describe the solution you'd like

Right now you can do ```unison:error to indicate that a block is supposed to result in an error. What I would also like is a way to indicate that a block would ideally succeed, but there is a not-yet-fixed bug that causes it to fail.

This is helpful for identifying when open issues are fixed indirectly. When a ticket is opened, we can add a known failure case to the repo immediately. When the issue is addressed – either directly or indirectly – that transcript will fail, forcing removal of the known-failure flag, and implying that the associated issue should be reviewed and either be closed or have a still-failing test case added.

I think that :error might be best used for this new purpose. This is so as issues are opened containing transcripts, they can be passing transcripts, and :error is probably the most obvious flag for contributors to use) while a less obvious flag like :intentional-error (open to suggestions[^1]) can be used for the relatively small number of cases where we have blocks where an error is the correct behavior.

[^1]: Maybe like :hide:all, :error:intentional?

Describe alternatives you've considered

As suggested in https://github.com/unisonweb/unison/pull/5263#issuecomment-2271642843, we could have another transcripts directory that holds known failures. I think this has two issues

  1. it’s too coarse – having multiple blocks that represent known failures in a single transcript allows for commentary attached to them, and allows issues to have only one such transcript.
  2. it’s also too coarse in that it doesn’t constrain where in the transcript the failure occurs, or what the failure is, which the current :error does nicely.
  3. contributors can’t specify it in issue transcripts (ideally we can just copy issue descriptions directly to transcript files unedited).

Additional context

Pay attention to #5214 and perhaps also #5199 when addressing this, as it would be good to minimize future flag name changes.

aryairani commented 2 months ago

Greg & I to follow up on the right next steps