stretchr
require: invalid examples from doc comments as functions don't return bool
Description
The doc comments for multiple functions in the require package have code examples that are invalid. Example:
https://github.com/stretchr/testify/blob/a53be35c3b0cfcd5189cffcfd75df60ea581104c/require/require.go#L1391
The example reads:
if require.NoError(t, err) {
require.Equal(t, expectedObj, actualObj)
}
but the NoError() function does not return any value, so this is invalid.
There are similar invalid examples in the doc comments for NoErrorf, NotEmpty and NotEmptyf.
Same as #1609 for require.Error but we missed some. The require documentation is generated from the assert documentation. We either need to make the assert documentation work for both packages or stop generating the one from the other.
Hello! I was wondering if this issue has already been resolved. Could you please confirm? Thank you!
https://github.com/stretchr/testify/pull/1675
Hello! I was wondering if this issue has already been resolved. Could you please confirm? Thank you!
No, #1675 fixed require.Error but not the other functions I've listed above.
I'd like to contribute to resolving this issue!
In my opinion, even though assert and require are similar, they have different responsibilities.
Wouldn't it be better to maintain separate documentation for each?
I've assigned you, @PCloud63514.
As with all things; maintaining separate docs is a trade-off. Most functions in require actually call their counterpart in assert, this ensures that they are identical. Most of the assert functions' docs convert cleanly to require. The error functions can have a doc example that converts cleanly, #1675 shows this. It's possible to move these functions to files outside of the generated files to have separate documentation, but we'd rather not maintain two identical sets of functions for these error assertions just to allow their docs to be different. It's also possible to do something else to use the same implementation but different docs, but in that case any update to the documentation might not notice that the require docs also need to be updated by hand.
I'll leave the implementation up to you. Propose a good trade-off and we'll consider it. Otherwise if you just update the docs to a version which converts cleanly I'd be happy merging that PR.
@brackendawson Sorry for the late reply. 😅 I’m not sure if I expressed my point clearly before.
My view is that the responsibility of 'require' is to fail immediately, not to return a value. Therefore, I believe updating the doc comments is the correct fix. For this purpose, I have added a doc generation method for the require package.
I’ve opened a PR with the revised form!
The more I think about it, the more I feel we should remove all code from comment and use Go Example.
They can also be generated, but they can be formatted
