Improve CI handling test coverage and added missing coverage test.

Signed-off-by: Lorenzo Buitizon <[email protected]>

Author: MrKeiKun

.github/workflows/ci-testing.yml

@@ -27,6 +27,23 @@ jobs:
             ${{ runner.os }}-go-
       - name: test
         run: make test
+      - name: Generate coverage report
+        run: go tool cover -func=coverage.out
+      - name: Generate HTML coverage report
+        run: go tool cover -html=coverage.out -o coverage.html
+      - name: Upload coverage reports as artifacts
+        uses: actions/upload-artifact@v4
+        with:
+          name: coverage-reports
+          path: |
+            coverage.out
+            coverage.html
+      - name: Upload coverage to Codecov
+        uses: codecov/codecov-action@v4
+        with:
+          file: ./coverage.out
+          flags: unittests
+          name: codecov-umbrella
 
   testacc:
     runs-on: ubuntu-22.04
@@ -61,3 +78,12 @@ jobs:
         run: make -e testacc
         env:
           NETBOX_VERSION: ${{ matrix.netbox-version }}
+      - name: Check coverage threshold
+        if: matrix.netbox-version == 'v4.2.9'  # Only check on one version to avoid duplication
+        run: |
+          COVERAGE=$(go tool cover -func=coverage.out | grep total | awk '{print substr($3, 1, length($3)-1)}')
+          echo "Current coverage: $COVERAGE%"
+          if (( $(echo "$COVERAGE < 75.0" | bc -l) )); then
+            echo "Coverage dropped below 75%: $COVERAGE%"
+            exit 1
+          fi

GNUmakefile

@@ -14,7 +14,7 @@ default: testacc
 .PHONY: testacc
 testacc: docker-up
 	@echo "⌛ Startup acceptance tests on $(NETBOX_SERVER_URL) with version $(NETBOX_VERSION)"
-	TF_ACC=1 go test -timeout 20m -v -cover $(TEST)
+	TF_ACC=1 go test -timeout 20m -v -cover -coverprofile=coverage.out $(TEST)
 
 .PHONY: testacc-specific-test
 testacc-specific-test: # docker-up
@@ -24,7 +24,7 @@ testacc-specific-test: # docker-up
 
 .PHONY: test
 test:
-	go test $(TEST) $(TESTARGS) -timeout=120s -parallel=4 -cover
+	go test $(TEST) $(TESTARGS) -timeout=120s -parallel=4 -cover -coverprofile=coverage.out
 
 # Run dockerized Netbox for acceptance testing
 .PHONY: docker-up
@@ -52,3 +52,11 @@ docs:
 fmt:
 	go fmt
 	go fmt netbox/*.go
+
+.PHONY: coverage
+coverage:
+	./scripts/coverage.sh
+
+.PHONY: coverage-html
+coverage-html:
+	./scripts/coverage.sh html

docs/MOCK_TESTING.md

@@ -0,0 +1,130 @@
+# Mock-Based Testing for Better Coverage
+
+This document outlines how to implement mock-based unit tests to improve test coverage beyond acceptance tests.
+
+## Why Mock-Based Testing?
+
+- **Higher Coverage**: Test error paths and edge cases without live NetBox instance
+- **Faster Execution**: No network calls or Docker setup required
+- **Reliable**: Tests don't depend on external services
+- **Focused**: Test specific functions in isolation
+
+## Current Coverage Status
+
+Current test coverage: ~75.0%
+
+Areas that would benefit from mock tests:
+- API error handling in data sources and resources
+- Network failure scenarios
+- Authentication errors
+- Rate limiting
+- Malformed responses
+
+## Recommended Mock Testing Setup
+
+### 1. Choose a Mocking Framework
+
+```bash
+go get github.com/stretchr/testify/mock
+# or
+go get github.com/golang/mock/gomock
+```
+
+### 2. Example Mock Test Structure
+
+```go
+package netbox
+
+import (
+    "errors"
+    "testing"
+
+    "github.com/fbreckle/go-netbox/netbox/client"
+    "github.com/fbreckle/go-netbox/netbox/client/extras"
+    "github.com/fbreckle/go-netbox/netbox/models"
+    "github.com/stretchr/testify/assert"
+)
+
+// Mock client for testing
+type mockNetBoxClient struct {
+    extrasAPI *mockExtrasAPI
+}
+
+type mockExtrasAPI struct {
+    tagsListFunc func(*extras.ExtrasTagsListParams) (*extras.ExtrasTagsListOK, error)
+}
+
+func (m *mockExtrasAPI) ExtrasTagsList(params *extras.ExtrasTagsListParams, authInfo interface{}) (*extras.ExtrasTagsListOK, error) {
+    if m.tagsListFunc != nil {
+        return m.tagsListFunc(params)
+    }
+    return nil, errors.New("mock not implemented")
+}
+
+func TestFindTag_APIError(t *testing.T) {
+    // Setup mock
+    mockAPI := &mockExtrasAPI{
+        tagsListFunc: func(params *extras.ExtrasTagsListParams) (*extras.ExtrasTagsListOK, error) {
+            return nil, errors.New("connection refused")
+        },
+    }
+
+    mockClient := &client.NetBoxAPI{}
+    // Note: In practice, you'd need to properly inject the mock
+
+    // This is a simplified example - actual implementation would require
+    // interface extraction and dependency injection
+    tag, err := findTag(mockClient, "test-tag")
+
+    assert.Error(t, err)
+    assert.Nil(t, tag)
+    assert.Contains(t, err.Error(), "API Error")
+}
+```
+
+### 3. Implementation Strategy
+
+1. **Extract Interfaces**: Create interfaces for API clients to enable mocking
+2. **Dependency Injection**: Modify functions to accept interfaces instead of concrete types
+3. **Mock Generation**: Use code generation tools to create mocks
+4. **Test Organization**: Separate unit tests from acceptance tests
+
+### 4. Functions to Mock Test
+
+Priority order for mock testing:
+
+1. **Utility Functions** (already done)
+   - `findTag` in `tags.go`
+   - `getNestedTagListFromResourceDataSet`
+   - `readTags`
+
+2. **Data Source Functions**
+   - `dataSourceNetboxAsnsRead` - API errors, no results
+   - `dataSourceNetboxTagRead` - multiple results, API errors
+
+3. **Resource Functions**
+   - CRUD operations with API failures
+   - 404 handling in read/delete
+   - Validation errors
+
+4. **Provider Functions**
+   - `providerConfigure` with invalid credentials
+   - Version checking failures
+
+### 5. Benefits Expected
+
+- Coverage increase: 74.8% → 85%+
+- Faster test execution
+- Better error path testing
+- Reduced CI resource usage
+
+## Getting Started
+
+1. Start with simple functions like `findTag`
+2. Extract interfaces for API clients
+3. Use testify/mock for simple mocking
+4. Gradually expand to more complex scenarios
+
+## Example Implementation
+
+See `tags_mock_test.go` for a basic mock test example (requires interface extraction for full implementation).

netbox/client_test.go

@@ -62,6 +62,57 @@ func TestURLMissingAccessKey(t *testing.T) {
 	assert.Error(t, err)
 }
 
+func TestClientWithCustomHeaders(t *testing.T) {
+	config := Config{
+		APIToken:  "07b12b765127747e4afd56cb531b7bf9c61f3c30",
+		ServerURL: "https://localhost:8080",
+		Headers: map[string]interface{}{
+			"X-Custom-Header": "test-value",
+			"X-Another-Header": 123,
+		},
+	}
+
+	client, err := config.Client()
+	assert.NotNil(t, client)
+	assert.NoError(t, err)
+}
+
+func TestClientWithInsecureHTTPS(t *testing.T) {
+	config := Config{
+		APIToken:          "07b12b765127747e4afd56cb531b7bf9c61f3c30",
+		ServerURL:         "https://localhost:8080",
+		AllowInsecureHTTPS: true,
+	}
+
+	client, err := config.Client()
+	assert.NotNil(t, client)
+	assert.NoError(t, err)
+}
+
+func TestClientWithRequestTimeout(t *testing.T) {
+	config := Config{
+		APIToken:      "07b12b765127747e4afd56cb531b7bf9c61f3c30",
+		ServerURL:     "https://localhost:8080",
+		RequestTimeout: 30,
+	}
+
+	client, err := config.Client()
+	assert.NotNil(t, client)
+	assert.NoError(t, err)
+}
+
+func TestClientWithStripTrailingSlashes(t *testing.T) {
+	config := Config{
+		APIToken:                    "07b12b765127747e4afd56cb531b7bf9c61f3c30",
+		ServerURL:                   "https://localhost:8080/",
+		StripTrailingSlashesFromURL: true,
+	}
+
+	client, err := config.Client()
+	assert.NotNil(t, client)
+	assert.NoError(t, err)
+}
+
 func TestAdditionalHeadersSet(t *testing.T) {
 	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		vals, ok := r.Header["Hello"]
@@ -87,6 +138,32 @@ func TestAdditionalHeadersSet(t *testing.T) {
 	client.Status.StatusList(req, nil)
 }
 
+func TestCustomHeaderTransportRoundTrip(t *testing.T) {
+	ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		vals, ok := r.Header["Custom-Header"]
+
+		assert.True(t, ok)
+		assert.Len(t, vals, 1)
+		assert.Equal(t, vals[0], "test-value")
+		w.WriteHeader(http.StatusOK)
+	}))
+	defer ts.Close()
+
+	// Create a custom header transport
+	transport := customHeaderTransport{
+		original: http.DefaultTransport,
+		headers: map[string]interface{}{
+			"Custom-Header": "test-value",
+		},
+	}
+
+	req, _ := http.NewRequest("GET", ts.URL, nil)
+	resp, err := transport.RoundTrip(req)
+
+	assert.NoError(t, err)
+	assert.Equal(t, http.StatusOK, resp.StatusCode)
+}
+
 /* TODO
 func TestInvalidHttpsCertificate(t *testing.T) {}
 */

netbox/custom_fields_test.go

@@ -0,0 +1,30 @@
+package netbox
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestGetCustomFields(t *testing.T) {
+	// Test with valid map
+	input := map[string]interface{}{
+		"field1": "value1",
+		"field2": "value2",
+	}
+	result := getCustomFields(input)
+	assert.Equal(t, input, result)
+
+	// Test with empty map
+	emptyInput := map[string]interface{}{}
+	result2 := getCustomFields(emptyInput)
+	assert.Nil(t, result2)
+
+	// Test with nil
+	result3 := getCustomFields(nil)
+	assert.Nil(t, result3)
+
+	// Test with non-map type
+	result4 := getCustomFields("not a map")
+	assert.Nil(t, result4)
+}

netbox/data_source_netbox_asns_test.go

@@ -2,6 +2,7 @@ package netbox
 
 import (
 	"fmt"
+	"regexp"
 	"testing"
 
 	"github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource"
@@ -65,6 +66,26 @@ data "netbox_asns" "test" {
 }`
 }
 
+func testAccNetboxAsnsInvalidFilter() string {
+	return `
+data "netbox_asns" "test" {
+  filter {
+	name = "invalid_filter"
+	value = "test"
+  }
+}`
+}
+
+func testAccNetboxAsnsNoResults() string {
+	return `
+data "netbox_asns" "test" {
+  filter {
+	name = "asn"
+	value = "999999"
+  }
+}`
+}
+
 func TestAccNetboxAsnsDataSource_basic(t *testing.T) {
 	testName := testAccGetTestName("asns_ds_basic")
 	setUp := testAccNetboxAsnsSetUp(testName)
@@ -104,3 +125,27 @@ func TestAccNetboxAsnsDataSource_basic(t *testing.T) {
 		},
 	})
 }
+
+func TestAccNetboxAsnsDataSource_invalidFilter(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config:      testAccNetboxAsnsInvalidFilter(),
+				ExpectError: regexp.MustCompile("'invalid_filter' is not a supported filter parameter"),
+			},
+		},
+	})
+}
+
+func TestAccNetboxAsnsDataSource_noResults(t *testing.T) {
+	resource.Test(t, resource.TestCase{
+		Providers: testAccProviders,
+		Steps: []resource.TestStep{
+			{
+				Config:      testAccNetboxAsnsNoResults(),
+				ExpectError: regexp.MustCompile("no result"),
+			},
+		},
+	})
+}