search

sourcegraph / lsif-go

Language Server Indexing Format (LSIF) generator for Go
https://lsif.dev/
MIT License
116 stars 20 forks source link

Uncommitted API docs changes when running tests #202

Closed chrismwendt closed 3 years ago

chrismwendt commented 3 years ago
CleanShot 2021-09-22 at 09 52 33@2x

@slimsag Is this for golden tests? Checking generated code into the repo adds a bit of overhead while developing other features. Perhaps the API docs tests could generate in a tmp directory and perform assertions on that?

chrismwendt commented 3 years ago

We're working around this by disabling the API docs tests during development of https://github.com/sourcegraph/lsif-go/pull/199

CleanShot 2021-09-25 at 13 32 36@2x
slimsag commented 3 years ago

Sorry for the delay on this.

Yes, these are golden tests: the test is, given all of our testdata code / Go packages, what API docs would be generated?

If there is a diff, the tests should fail (but there may be a bug here where they don't). Regardless, if there are changes - either because we've introduced new code into the testdata Go packages, or because we've made changes, they are real changes to API docs too and should either be committed (if they look good) or reviewed by me (if you're unsure)

It's important that this runs over all code in the testdata directory, too, because we're constantly finding new edge cases that we haven't considered before and it's important we test API docs generation on those edge cases too.

chrismwendt commented 3 years ago

Thanks for the clarification! I actually didn't know the goal was to find more edge cases, so I wasn't even paying attention to the generated files.

It seems to me that there are 2 orthogonal goals we're pursuing:

I don't know enough about API docs to help you find edge cases, so I might actually end up committing bad goldens 😖 (fool's gold 😸)

In the other direction, if find-implementations were to generate some golden data (probably not, but who knows), I'd venture to guess that you don't know enough about find-implementations to vet the goldens.

The end result of crossing into each others' realm of expertise is a false sense of coverage and more confusing debugging later when we notice bad goldens were committed.

Associating the testdata with the relevant tests would resolve this. I'm happy to do that, and in the meantime let me know if you have another idea 💡