Increase Unit Test Coverage by ChristianAtDell · Pull Request #54 · dell/gocsi

ChristianAtDell · 2025-01-29T14:27:07Z

Description

This PR includes substantial unit test coverage increase across gocsi packages.

GitHub Issues

List the GitHub issues impacted by this PR:

GitHub Issue #
https://github.com/dell/csm/issues/1707

Checklist:

I have performed a self-review of my own code to ensure there are no formatting, vetting, linting, or security issues
I have verified that new and existing unit tests pass locally with my changes
I have not allowed coverage numbers to degenerate
I have maintained at least 80% code coverage
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have added tests that prove my fix is effective or that my feature works
Backward compatibility is not broken

How Has This Been Tested?

opensuse154:~/git/gocsi # go test -cover -race ./...
?       github.com/dell/gocsi/middleware/serialvolume/types     [no test files]
        github.com/dell/gocsi/mock/provider             coverage: 0.0% of statements
        github.com/dell/gocsi/mock              coverage: 0.0% of statements
        github.com/dell/gocsi/mock/service              coverage: 0.0% of statements
ok      github.com/dell/gocsi   6.100s  coverage: 80.3% of statements
ok      github.com/dell/gocsi/context   1.055s  coverage: 82.1% of statements
ok      github.com/dell/gocsi/csc       1.029s
ok      github.com/dell/gocsi/csc/cmd   1.111s  coverage: 82.2% of statements
ok      github.com/dell/gocsi/middleware/logging        1.075s  coverage: 82.4% of statements
ok      github.com/dell/gocsi/middleware/requestid      1.092s  coverage: 82.8% of statements
ok      github.com/dell/gocsi/middleware/serialvolume   1.080s  coverage: 91.9% of statements
ok      github.com/dell/gocsi/middleware/serialvolume/etcd      9.454s  coverage: 80.3% of statements
ok      github.com/dell/gocsi/middleware/specvalidator  1.084s  coverage: 80.7% of statements
ok      github.com/dell/gocsi/testing   40.879s coverage: [no statements]
ok      github.com/dell/gocsi/utils     1.254s  coverage: 80.4% of statements

The only packages not at 80% or above are:

middleware/serialvolume/types (no functional code, only type definitions)
mock, mock/service, mock/provider (mock controller client/server, for testing)
testing (legacy gomega unit tests)

All other packages are at 80% coverage or above.

…to pub/add-unit-tests

…subpackage WIP.

…to pub/add-unit-tests

alexemc · 2025-02-14T17:08:20Z

+	// if CTX has this key, we want to return error for UT
+	if ctx.Value(ContextKey("returnError")) == "true" {
+		return nil, status.Error(codes.InvalidArgument, "Returned error from mock CreateVolume")
+	}


Is it really needed? You can induce an error by passing a bad req, e.g. req.Name longer than 128 characters.

This is in the mock, not the driver code; I don't see a problem with having a ctx variable for returning an error, though there are potentially more elegant ways to go about it. I will see if I can find a way to inject this error otherwise and ask @JacobGros since he designed the test.

I decided on a uniform approach for forcing errors in the mocks. Some CSI requests (like ControllerGetCapabilities) do not have any parameters, others would require a different parameter configuration to get the error to return. By setting it in the context, we have an approach that works for all mock CSI calls

ChristianAtDell and others added 30 commits January 21, 2025 10:51

new branch for unit testing

c2e8213

boost middleware/logging to 20.6%

c3981b4

add test to serialvolume

ba6430c

first Cobra test

89185b2

Merge branch 'pub/add-unit-tests' of https://github.com/dell/gocsi in…

0651daf

…to pub/add-unit-tests

start etcd

7305486

update go.mod

63d788a

etcd 80 coverage

0366911

increase middleware/logging coverage to 82%

0e1737e

controller test fixes

9608787

Merge branch 'pub/add-unit-tests' of https://github.com/dell/gocsi in…

f276084

…to pub/add-unit-tests

start specvalidator

69b7202

specvalidator 80

2d0af7a

create volume + create snapshot tests

a2e1a90

Merge branch 'pub/add-unit-tests' of https://github.com/dell/gocsi in…

e98846e

…to pub/add-unit-tests

add first node tests, csc/cmd at 35.9%

5baa85f

add the first node tests

e8cf8c8

test coverage 82.8%

ee89bbd

add csc/cmd/identity tests

d31237c

controller cmd tests added

b84cf41

Merge branch 'pub/add-unit-tests' of https://github.com/dell/gocsi in…

5132fe5

…to pub/add-unit-tests

abstracted root setup in tests

38987ae

test context

66f500c

add more node function testing, coverage of csc/cmd at 68.6%

afca2f3

push csc/cmd coverage to 70%

a2b07ca

add 80 coverage

1b6df43

vartypes tests

94c000f

Merge branch 'pub/add-unit-tests' of https://github.com/dell/gocsi in…

020dd9a

…to pub/add-unit-tests

standardized suite setup in identity test

161d2cd

some node setup issues resolved

9c38b53

JacobGros and others added 15 commits February 5, 2025 15:53

add ctx value to test failing CSI calls

5604e2b

boost coverage of csc/cmd to 86.3%

b657fa8

push csc/cmd to 92.4%

f111bd0

Increased unit-test coverage for the context package to 96.4%

e8afb58

Fixed bad endpoint socket file name check when running tests in Windows.

d2588d4

Increased the util package test coverage to 91.3%

7c3f0da

boost gocsi coverage to 86.1%

7a7da57

middleware package test coverage increased to 90.7% overall, logging …

becc5d0

…subpackage WIP.

Merge branch 'pub/add-unit-tests' of https://github.com/dell/gocsi in…

725a229

…to pub/add-unit-tests

logging package test coverage increased to 100%

e4d37a1

remove dead code

c932de4

gofumpt and boost gocsi coverage to 90.1%

b3e5826

golangci-lint

8a3fdd8

golangci-lint

9618db9

Merge branch 'main' into pub/add-unit-tests

c42824e

alikdell previously approved these changes Feb 14, 2025

View reviewed changes

alexemc reviewed Feb 14, 2025

View reviewed changes

PR feedback

137d7c6

JacobGros dismissed alikdell’s stale review via 137d7c6 February 17, 2025 14:35

ChristianAtDell and others added 5 commits February 17, 2025 09:49

PR feedback

849cead

Update common-workflows.yaml

ec2258f

Update common-workflows.yaml

14b493f

Update common-workflows.yaml

7481b36

Update common-workflows.yaml

b782eda

alikdell approved these changes Feb 17, 2025

View reviewed changes

donatwork approved these changes Feb 17, 2025

View reviewed changes

ChristianAtDell changed the base branch from main to pub/ut-intermediate February 17, 2025 22:45

ChristianAtDell merged commit f9e7c28 into pub/ut-intermediate Feb 17, 2025

ChristianAtDell deleted the pub/add-unit-tests branch February 17, 2025 22:46

ChristianAtDell mentioned this pull request Feb 17, 2025

Increase Unit Test Coverage #64

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Increase Unit Test Coverage#54

Increase Unit Test Coverage#54
ChristianAtDell merged 58 commits into
pub/ut-intermediatefrom
pub/add-unit-tests

ChristianAtDell commented Jan 29, 2025

Uh oh!

Uh oh!

alexemc Feb 14, 2025

Uh oh!

ChristianAtDell Feb 17, 2025

Uh oh!

JacobGros Feb 17, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

Conversation

ChristianAtDell commented Jan 29, 2025

Description

GitHub Issues

Checklist:

How Has This Been Tested?

Uh oh!

Uh oh!

alexemc Feb 14, 2025

Choose a reason for hiding this comment

Uh oh!

ChristianAtDell Feb 17, 2025

Choose a reason for hiding this comment

Uh oh!

JacobGros Feb 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants