Add error checking in tests and enable errcheck there #1980
Conversation
Note, claude did it because it followed the existing patttern in this function.
denik
changed the title
| exclude-rules: | ||
| - path-except: (_test\.go|internal) | ||
| linters: | ||
| - errcheck |
There was a problem hiding this comment.
This means it is enabled only for tests?
There was a problem hiding this comment.
Yes, it's enabled if path matches this regex (_test.go|internal). It's not obvious from reading the config but I tested that it works like that.
| _, err := t.stdinW.Write([]byte(text + "\n")) | ||
| if err != nil { | ||
| panic("Failed to to write to t.stdinW") | ||
| } |
There was a problem hiding this comment.
t embeds *testing.T, so you can use require.
There was a problem hiding this comment.
That file uses panic() in 2 other places, so I'm just following how it's done here. We can update all at once later.
bundle/config/root_test.go
Outdated
| @@ -156,7 +156,8 @@ func TestRootMergeTargetOverridesWithVariables(t *testing.T) { | |||
| }, | |||
| }, | |||
| } | |||
| root.initializeDynamicValue() | |||
| require.NoError(t, root.initializeDynamicValue()) | |||
|
|
|||
|
If integration tests don't run automatically, an authorized user can run them manually by following the instructions below: Trigger: Inputs:
Checks will be approved automatically on success. |
|
Test Details: go/deco-tests/12234884650 |
|
Merging -there was one integration test failing, due to API quotas. |
Changes
Fix all errcheck-found issues in tests and test helpers. Mostly this done by adding require.NoError(t, err), sometimes panic() where t object is not available).
Initial change is obtained with aider+claude, then manually reviewed and cleaned up.
Tests
Existing tests.