-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Update module github.com/vektra/mockery/v2 to v3 #7051
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: anmol7344 <[email protected]>
@yurishkuro I have tried to resolve some of the failing unit tests but I was unable to resolve some of the unit tests as I was unable to understand what was failing |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7051 +/- ##
==========================================
- Coverage 96.04% 96.03% -0.02%
==========================================
Files 352 355 +3
Lines 20821 20973 +152
==========================================
+ Hits 19998 20141 +143
- Misses 620 626 +6
- Partials 203 206 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
a couple of mismatched expectations are failing, look rather straightforward to fix. |
Signed-off-by: anmol7344 <[email protected]>
I had a doubt that now we want to migrate to v3 which is more strict than v2 so would that not make writing or maintaining unit tests more difficult? |
cmd/query/app/apiv3/gateway_test.go
Outdated
gw.reader. | ||
On("GetTraces", matchContext, query). | ||
On("GetTraces", matchContext, mock.AnythingOfType("[]tracestore.GetTraceParams")). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why match in string name? The error was that the function was expecting an array, so this should've worked:
query := []tracestore.GetTraceParams{{TraceID: traceID}}
gw.reader.
On("GetTraces", matchContext, query).
@@ -127,7 +127,7 @@ func TestGetTrace(t *testing.T) { | |||
|
|||
func TestGetTraceStorageError(t *testing.T) { | |||
tsc := newTestServerClient(t) | |||
tsc.reader.On("GetTraces", matchContext, tracestore.GetTraceParams{TraceID: traceID}). | |||
tsc.reader.On("GetTraces", matchContext, mock.AnythingOfType("[]tracestore.GetTraceParams")). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pass actual array with trace ID
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
your changes are making the tests less strict. There is no reason to do that. The errors were about expecting arrays when we were passing a single value, so just pass arrays. Basically our old tests were not adhering to the actual APIs of the mocks and we should fix that, not water down the tests.
Signed-off-by: Yuri Shkuro <[email protected]>
please see b0c5d04 for a sample of how tests can be fixed, without watering them down by any-matchers. |
Signed-off-by: anmol7344 <[email protected]>
Thanks! |
## Which problem is this PR solving? - Resolves jaegertracing#7008 ## Description of the changes - Continuation of pr jaegertracing#7042 ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: anmol7344 <[email protected]> Signed-off-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]>
## Which problem is this PR solving? - Resolves jaegertracing#7008 ## Description of the changes - Continuation of pr jaegertracing#7042 ## Checklist - [x] I have read https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md - [x] I have signed all commits - [x] I have added unit tests for the new functionality - [x] I have run lint and test steps successfully - for `jaeger`: `make lint test` - for `jaeger-ui`: `npm run lint` and `npm run test` --------- Signed-off-by: anmol7344 <[email protected]> Signed-off-by: Yuri Shkuro <[email protected]> Co-authored-by: Yuri Shkuro <[email protected]> Signed-off-by: amol-verma-allen <[email protected]>
Which problem is this PR solving?
Description of the changes
Checklist
jaeger
:make lint test
jaeger-ui
:npm run lint
andnpm run test