doc: document Start and Await methods and mocks #14375
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #14375 +/- ##
==========================================
- Coverage 93.06% 93.06% -0.01%
==========================================
Files 2191 2191
Lines 193203 193210 +7
==========================================
+ Hits 179812 179814 +2
- Misses 13391 13396 +5 ☔ View full report in Codecov by Sentry. |
| @@ -6,6 +6,17 @@ breaking changes in the upcoming 3.x release. This release is scheduled for | |||
|
|
|||
| ## v2.26.0 - TBD | |||
|
|
|||
| ### BREAKING TESTING CHANGES | |||
There was a problem hiding this comment.
nit: doesn't matter, but line break?
CHANGELOG.md
Outdated
| @@ -6,6 +6,17 @@ breaking changes in the upcoming 3.x release. This release is scheduled for | |||
|
|
|||
| ## v2.26.0 - TBD | |||
|
|
|||
| ### BREAKING TESTING CHANGES | |||
| If you don't have tests that `EXPECT_CALL` on methods in `MockConnection` that | |||
There was a problem hiding this comment.
FTR, ON_CALL would be ambiguous too. Maybe:
If you don't mock Long Running Operations (LRO) in your tests, these changes will not affect you.
CHANGELOG.md
Outdated
| for the preexisting LRO methods have been expanded. Uses of `EXPECT_CALL` that | ||
| do not have matchers for the arguments will be ambiguous. To quickly remedy this | ||
| change instances of `EXPECT_CALL(*mock, Method)` to | ||
| `EXPECT_CALL(*mock, Method(_))`. |
There was a problem hiding this comment.
optional: we wouldn't write this, but maybe s/Method(_)/Method(::testing::_)/ for those who are not familiar with the placeholder.
| @@ -37,6 +37,16 @@ std::string FormatMethodComments( | |||
| */ | |||
| bool CheckMethodCommentSubstitutions(); | |||
|
|
|||
| /** | |||
| * Comments for LRO Start overload. | |||
| "\n // Due to additional overloads for this method\n" | ||
| " // EXPECT_CALL(*mock, $method_name$) is now ambiguous. Use\n" | ||
| " // EXPECT_CALL(*mock, $method_name$(_)) instead.\n"}, |
There was a problem hiding this comment.
We should use doxygen style comments /// and put the code in code quotes.
And optionally, spell out ::testing::_.
There was a problem hiding this comment.
Reviewable status: 0 of 527 files reviewed, 3 unresolved discussions (waiting on @dbolduc)
a discussion (no related file):
Previously, dbolduc (Darren Bolduc) wrote…
nit: doesn't matter, but line break?
Done
a discussion (no related file):
Previously, dbolduc (Darren Bolduc) wrote…
FTR,
ON_CALLwould be ambiguous too. Maybe:If you don't mock Long Running Operations (LRO) in your tests, these changes will not affect you.
Done
a discussion (no related file):
Previously, dbolduc (Darren Bolduc) wrote…
optional: we wouldn't write this, but maybe
s/Method(_)/Method(::testing::_)/for those who are not familiar with the placeholder.
Done.
a discussion (no related file):
Previously, dbolduc (Darren Bolduc) wrote…
nit: extra space
Done
There was a problem hiding this comment.
Reviewable status: 0 of 527 files reviewed, 3 unresolved discussions (waiting on @dbolduc)
generator/internal/mock_connection_generator.cc line 109 at r1 (raw file):
Previously, dbolduc (Darren Bolduc) wrote…
We should use doxygen style comments
///and put the code in code quotes.And optionally, spell out
::testing::_.
Done.
part of the work for #7658
This change is