-
Not necessarily representing the actions or views of anyone even when stated otherwise. | Profile pic CC-BY 4.0: ESA/Hubble
- Joined on
2024-05-21 -
-
Move these into run() and remove the global calls, to not duplicate them from main.go and actually test them?
Filed feature request for mock expect reason: https://github.com/uber-go/mock/issues/296
To be more specific expected mocks by default are Times(1). You are right that those where EXPECT() is never called will fail the test if called, but here expect was called.
While too many mocks make things harder, having fewer of them does not solve knowing when a mock is intended to test something by itself vs just enabling a later test.
So where is the test that…
No, by default mocks are Times(1) and AnyTimes() overwrites that to 0-veryBigNumber, so here it allows UpdatePullRequest to be called.
Should be Times(1). Possibly compare that the right the label is set?
For future improvement: This and the surrounding comments should make clear why the code is in a comment here.
I don't think that is possible? LLM or human mistake? If I didn't miss anything delete it.
The commit message says "PR events correctly trigger processing", but as all these allow anything, so do these actually do that?
Most of these _ are wrong. The LLM got confused by the error when running the test?
For future: In tests with many expected functions, it is difficult to spot which mock expectation is important to the test and which ones are just to define required return values. What is the best way? I'm not sure. What do you think?
Looks good. One change can be done in a later PR. Though, I have not carefully reviewed ever line.