*: add more unit tests #4017

coderabbitai · 2026-01-28T09:49:33Z

⚠️ Potential issue | 🟡 Minor

Handle restClient errors before dereferencing.

If restClient fails, c may be nil and cause a panic.

✅ Proposed fix

- c, _ := restClient(testServer) + c, err := restClient(testServer) + require.NoError(t, err) + require.NotNil(t, c)

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

c, _ := restClient(testServer)

req := c.Post()

require.NotNil(t, req)

req = c.Get()

require.NotNil(t, req)

req = c.Put()

require.NotNil(t, req)

req = c.Delete()

require.NotNil(t, req)

c, err := restClient(testServer)

require.NoError(t, err)

require.NotNil(t, c)

req := c.Post()

require.NotNil(t, req)

req = c.Get()

require.NotNil(t, req)

req = c.Put()

require.NotNil(t, req)

req = c.Delete()

require.NotNil(t, req)

🤖 Prompt for AI Agents

In `@pkg/api/internal/rest/client_test.go` around lines 90 - 102, The test dereferences c returned from restClient without checking its error, which can panic if restClient fails; change the call to capture the error (e.g., c, err := restClient(testServer)) and assert failure if err != nil using require.NoError(t, err) or require.NotNil(t, c) before calling c.Post/Get/Put/Delete so the test fails cleanly instead of panicking.

-Original file line number
+Diff line change
@@ Expand Up / @@ -17,7 +17,7 @@ import ( @@
     	"fmt"
     	"testing"
-    	"github.com/pingcap/tiflow/pkg/errors"
+    	"github.com/pingcap/ticdc/pkg/errors"
     	"github.com/stretchr/testify/require"
     )
@@ Expand Down Expand Up / @@ -274,10 +274,10 @@ func TestInvalidExpression(t *testing.T) { @@
     	require.ErrorContains(t, err, invalidExpr)
     }
-    // cmd: go test -run='^$' -bench '^(BenchmarkSubstitute)$' github.com/pingcap/tiflow/cdc/sink/dispatcher/topic
+    // cmd: go test -run='^$' -bench '^(BenchmarkSubstitute)$' github.com/pingcap/ticdc/cdc/sink/dispatcher/topic
     // goos: linux
     // goarch: amd64
-    // pkg: github.com/pingcap/tiflow/cdc/sink/dispatcher
+    // pkg: github.com/pingcap/ticdc/cdc/sink/dispatcher
     // cpu: Intel(R) Xeon(R) CPU E5-2630 v4 @ 2.20GHz
     // BenchmarkSubstitute/schema_substitution-40         	  199372	      6477 ns/op
     // BenchmarkSubstitute/schema_table_substitution-40   	  110752	     13637 ns/op
@@ Expand Down @@

-Original file line number
+Diff line change
@@ -0,0 +1,91 @@
+    // Copyright 2021 PingCAP, Inc.
+    //
+    // Licensed under the Apache License, Version 2.0 (the "License");
+    // you may not use this file except in compliance with the License.
+    // You may obtain a copy of the License at
+    //
+    //     http://www.apache.org/licenses/LICENSE-2.0
+    //
+    // Unless required by applicable law or agreed to in writing, software
+    // distributed under the License is distributed on an "AS IS" BASIS,
+    // See the License for the specific language governing permissions and
+    // limitations under the License.
+    package rest
+    import (
+    	"testing"
+    	"github.com/pingcap/ticdc/pkg/security"
+    	"github.com/stretchr/testify/require"
+    )
+    func TestCDCRESTClientCommonConfigs(t *testing.T) {
+    	_, err := CDCRESTClientFromConfig(&Config{Host: "127.0.0.1"})
+    	require.NotNil(t, err)
+    	_, err = CDCRESTClientFromConfig(&Config{Host: "127.0.0.1", Version: "v1"})
+    	require.NotNil(t, err)
+    	_, err = CDCRESTClientFromConfig(&Config{Host: "127.0.0.1", APIPath: "/api"})
+    	require.NotNil(t, err)
+    	_, err = CDCRESTClientFromConfig(&Config{Host: "http://127.0.0.1:2379", APIPath: "/api", Version: "v1"})
+    	require.Nil(t, err)
+    	_, err = CDCRESTClientFromConfig(&Config{Host: "127.0.0.1:2379", APIPath: "/api", Version: "v2"})
+    	require.Nil(t, err)
+    }
+    func checkTLS(config *Config) bool {
+    	baseURL, _, err := defaultServerURLFromConfig(config)
+    	if err != nil {
+    		return false
+    	}
+    	return baseURL.Scheme == "https"
+    }
+    func TestCDCRESTClientUsingTLS(t *testing.T) {
+    	testCases := []struct {
+    		Config   *Config
+    		UsingTLS bool
+    	}{
+    		{
+    			Config:   &Config{},
+    			UsingTLS: false,
+    		},
+    		{
+    			Config: &Config{
+    				Host: "https://127.0.0.1",
+    			},
+    			UsingTLS: true,
+    		},
+    		{
+    			Config: &Config{
+    				Host: "127.0.0.1",
+    				Credential: &security.Credential{
+    					CAPath:   "foo",
+    					CertPath: "bar",
+    					KeyPath:  "test",
+    				},
+    			},
+    			UsingTLS: true,
+    		},
+    		{
+    			Config: &Config{
+    				Host: "///:://127.0.0.1",
+    				Credential: &security.Credential{
+    					CAPath:   "foo",
+    					CertPath: "bar",
+    					KeyPath:  "test",
+    				},
+    			},
+    			UsingTLS: false,
+    		},
+    	}
+    	for _, tc := range testCases {
+    		usingTLS := checkTLS(tc.Config)
+    		require.Equal(t, usingTLS, tc.UsingTLS)
+    	}
+    }

-Original file line number
+Diff line change
@@ -0,0 +1,171 @@
+    // Copyright 2021 PingCAP, Inc.
+    //
+    // Licensed under the Apache License, Version 2.0 (the "License");
+    // you may not use this file except in compliance with the License.
+    // You may obtain a copy of the License at
+    //
+    //     http://www.apache.org/licenses/LICENSE-2.0
+    //
+    // Unless required by applicable law or agreed to in writing, software
+    // distributed under the License is distributed on an "AS IS" BASIS,
+    // See the License for the specific language governing permissions and
+    // limitations under the License.
+    package rest
+    import (
+    	"bytes"
+    	"context"
+    	"errors"
+    	"io"
+    	"net/http"
+    	"net/http/httptest"
+    	"net/url"
+    	"strings"
+    	"testing"
+    	"time"
+    	"github.com/pingcap/ticdc/pkg/httputil"
+    	"github.com/stretchr/testify/require"
+    )
+    func TestRequestParams(t *testing.T) {
+    	req := (&Request{}).WithParam("foo", "bar")
+    	require.Equal(t, req.params, url.Values{"foo": []string{"bar"}})
+    	req.WithParam("hello", "world")
+    	require.Equal(t, req.params, url.Values{"foo": []string{"bar"}, "hello": []string{"world"}})
+    }
+    func TestRequestURI(t *testing.T) {
+    	req := (&Request{}).WithParam("foo", "bar").WithPrefix("test")
+    	req.WithURI("/production?foo=hello&val=1024")
+    	require.Equal(t, req.pathPrefix, "test/production")
+    	require.Equal(t, req.params, url.Values{"foo": []string{"hello"}, "val": []string{"1024"}})
+    }
+    type testStruct struct {
+    	Foo string `json:"foo"`
+    	Bar int    `json:"bar"`
+    }
+    func TestRequestBody(t *testing.T) {
+    	// test unsupported data type
+    	req := (&Request{}).WithBody(func() {})
+    	require.NotNil(t, req.err)
+    	require.Nil(t, req.body)
+    	// test data type which can be json marshalled
+    	p := &testStruct{Foo: "hello", Bar: 10}
+    	req = (&Request{}).WithBody(p)
+    	require.Nil(t, req.err)
+    	require.NotNil(t, req.body)
+    	// test data type io.Reader
+    	req = (&Request{}).WithBody(bytes.NewReader([]byte(`{"hello": "world"}`)))
+    	require.Nil(t, req.err)
+    	require.NotNil(t, req.body)
+    }
+    type clientFunc func(req *http.Request) (*http.Response, error)
+    func (f clientFunc) RoundTrip(req *http.Request) (*http.Response, error) {
+    	return f(req)
+    }
+    func TestRequestHeader(t *testing.T) {
+    	cli := httputil.NewTestClient(clientFunc(func(req *http.Request) (*http.Response, error) {
+    		require.Equal(t, req.Header.Get("signature"), "test-header1")
+    		return &http.Response{
+    			StatusCode: http.StatusOK,
+    			Body:       io.NopCloser(bytes.NewReader([]byte{})),
+    		}, nil
+    	}))
+    	req := newRequestWithClient(&url.URL{Path: "/test"}, "", nil).WithMethod(HTTPMethodGet)
+    	req.WithHeader("signature", "test-header2")
+    	req.WithHeader("signature", "test-header1")
+    	req.c.Client = cli
+    	_ = req.Do(context.Background())
+    }
+    func TestRequestDoContext(t *testing.T) {
+    	received := make(chan struct{})
+    	blocked := make(chan struct{})
+    	testServer := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+    		close(received)
+    		<-blocked
+    		rw.WriteHeader(http.StatusOK)
+    	}))
+    	defer testServer.Close()
+    	defer close(blocked)
+    	ctx, cancel := context.WithCancel(context.Background())
+    	go func() {
+    		<-received
+    		cancel()
+    	}()
+    	c, err := CDCRESTClientFromConfig(&Config{
+    		Host:    testServer.URL,
+    		APIPath: "/api",
+    		Version: "v1",
+    	})
+    	require.Nil(t, err)
+    	err = c.Get().
+    		WithPrefix("/test").
+    		WithTimeout(time.Second).
+    		Do(ctx).
+    		Error()
+    	require.NotNil(t, err)
+    }
+    func TestRequestDoContextTimeout(t *testing.T) {
+    	testServer := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, r *http.Request) {
+    		time.Sleep(200 * time.Millisecond)
+    		rw.WriteHeader(http.StatusOK)
+    	}))
+    	defer testServer.Close()
+    	ctx, cancel := context.WithCancel(context.Background())
+    	defer cancel()
+    	c, err := CDCRESTClientFromConfig(&Config{
+    		Host:    testServer.URL,
+    		APIPath: "/api",
+    		Version: "v1",
+    	})
+    	require.Nil(t, err)
+    	err = c.Get().
+    		WithPrefix("/test").
+    		WithTimeout(50 * time.Millisecond).
+    		Do(ctx).
+    		Error()
+    	require.NotNil(t, err)
+    }
+    func TestResultIntoError(t *testing.T) {
+    	result := Result{err: errors.New("test-error")}
+    	err := result.Into(&testStruct{})
+    	require.Equal(t, result.err, err)
+    	result = Result{
+    		body: []byte(`{"foo": "hello", "bar": 10}`),
+    	}
+    	var res testStruct
+    	err = result.Into(&res)
+    	require.Nil(t, err)
+    	require.Equal(t, res.Foo, "hello")
+    	require.Equal(t, res.Bar, 10)
+    }
+    func TestResultZeroLengthBody(t *testing.T) {
+    	result := Result{
+    		body: []byte{},
+    	}
+    	err := result.Into(&testStruct{})
+    	require.NotNil(t, err)
+    	require.Equal(t, strings.Contains(err.Error(), "0-length"), true)
+    }

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: add more unit tests #4017

Uh oh!

Diff view

Diff view

There are no files selected for viewing

coderabbitai bot Jan 28, 2026

Uh oh!

Uh oh!

Uh oh!

*: add more unit tests #4017

Are you sure you want to change the base?

Uh oh!

*: add more unit tests #4017

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

coderabbitai bot Jan 28, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!