testifylint: enable require-error #5034
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #5034 +/- ##
===========================================
+ Coverage 52.59% 95.61% +43.01%
===========================================
Files 145 319 +174
Lines 8599 18794 +10195
===========================================
+ Hits 4523 17970 +13447
+ Misses 3728 661 -3067
+ Partials 348 163 -185
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Signed-off-by: Matthieu MOREL <[email protected]>
yurishkuro
added
the
changelog:test
|
this one is impossible to review manually, can you describe how the PR was made? Are there any manual changes, or is everything just s/assert/require/? |
There was a problem hiding this comment.
Except the two quoted changes in this comments, this replaces all assert.{NoError,Error,ErrorContains,ErrorIs,EqualError} methods by require.{NoError,Error,ErrorContains,ErrorIs,EqualError}
Would you prefer to split this PR by package of around 30 files maybe ?
| @@ -53,21 +53,18 @@ func TestTraceIDMarshalJSONPB(t *testing.T) { | |||
| ref := model.SpanRef{TraceID: model.NewTraceID(testCase.hi, testCase.lo)} | |||
| out := new(bytes.Buffer) | |||
| err := new(jsonpb.Marshaler).Marshal(out, &ref) | |||
| if assert.NoError(t, err) { | |||
There was a problem hiding this comment.
This is the first kind of adaptation I had to do .
Remove the if and use a require instead.
This happens 6 times and only in this file
There was a problem hiding this comment.
yeah, in this case it's safe to replace it like this, but it's not always the case.
| @@ -607,14 +607,14 @@ func BenchmarkServiceIndexLimitFetch(b *testing.B) { | |||
|
|
|||
| // Opens a badger db and runs a test on it. | |||
| func runLargeFactoryTest(tb testing.TB, test func(tb testing.TB, sw spanstore.Writer, sr spanstore.Reader)) { | |||
| assert := assert.New(tb) | |||
| assertion := require.New(tb) | |||
There was a problem hiding this comment.
I also created require assertions instead of assert .
Which problem is this PR solving?
Description of the changes
How was this change tested?
Checklist
jaeger:make lint testjaeger-ui:yarn lintandyarn test