MF-1718 - Use static code analysis in CI #1729

AryanGodara · 2023-02-22T10:52:17Z

What does this do?

This PR makes the golangci-lint test pass.

Which issue(s) does this PR fix/relate to?

Resolves #1718

List any changes that modify/break current functionality

Behaviour of some functions has changed, and has to be discussed before finalising

Have you included tests for your changes?

No new tests

Did you document any new/modified functionality?

N/A

Notes

N/A

auth/api/grpc/setup_test.go

things/api/auth/grpc/setup_test.go

codecov-commenter · 2023-04-01T21:56:48Z

Codecov Report

Merging #1729 (39739e0) into master (ffd67ae) will decrease coverage by 0.24%.
The diff coverage is 45.67%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master    #1729      +/-   ##
==========================================
- Coverage   70.60%   70.37%   -0.24%     
==========================================
  Files         146      146              
  Lines       11421    11425       +4     
==========================================
- Hits         8064     8040      -24     
- Misses       2716     2733      +17     
- Partials      641      652      +11

Impacted Files	Coverage Δ
auth/keys.go	`100.00% <ø> (ø)`
auth/postgres/groups.go	`61.52% <0.00%> (-0.81%)`	⬇️
auth/service.go	`74.39% <0.00%> (ø)`
bootstrap/service.go	`83.10% <ø> (ø)`
lora/adapter.go	`60.00% <0.00%> (ø)`
pkg/sdk/go/keys.go	`0.00% <ø> (ø)`
readers/mongodb/messages.go	`85.36% <0.00%> (-2.29%)`	⬇️
readers/postgres/messages.go	`83.49% <0.00%> (-1.80%)`	⬇️
readers/timescale/messages.go	`83.00% <0.00%> (-1.85%)`	⬇️
things/postgres/things.go	`67.34% <0.00%> (-1.13%)`	⬇️
... and 14 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

auth/api/grpc/endpoint.go

tools/mqtt-bench/results.go

tools/mqtt-bench/bench.go

dborovcanin

Please update the linter version in CI script and extract it to the variable (like GOLANGCI_LINT_VERSION=v1.52.2). Also, please check the linter config here and find the best combination of linters for us.

AryanGodara · 2023-04-03T13:56:22Z

Please update the linter version in CI script and extract it to the variable (like GOLANGCI_LINT_VERSION=v1.52.2). Also, please check the linter config here and find the best combination of linters for us.

Removing deadcode as it's abandoned by the user, and is replaced by unused
Adding errcheck to make sure no error is ignored (unless by choice)
goconst can be useful -> Find repeated strings that can be replaced with constant

These two can be useful :-
unconvert -> Reports unnecessary type conversions
unparam -> Reports unused function parameters
ineffassign -> check ineffectual assignments (May cause lots of failed CIs as this is very strict checker)

auth/api/grpc/setup_test.go

auth/postgres/groups.go

readers/mongodb/messages.go

readers/timescale/messages.go

things/postgres/things.go

tools/mqtt-bench/bench.go

lora/mqtt/sub.go

opcua/gopcua/subscribe.go

readers/cassandra/messages.go

readers/mocks/messages.go

readers/postgres/messages.go

things/redis/streams.go

tools/mqtt-bench/bench.go

auth/api/grpc/setup_test.go

dborovcanin · 2023-04-07T15:22:13Z

cmd/certs/main.go

@@ -169,13 +169,14 @@ func loadCertificates(conf config, logger mflog.Logger) (tls.Certificate, *x509.
 	}

 	block, _ := pem.Decode(b)
-	if block == nil {
+	switch block {


Using if in this case is fine, preferable even. What was linter's remark?

It was not happy with the block == nil {}, so I had to change the format to reverse the check to block != nil

Can you, please, send me the exact golangci-lint command to test this? The command we have in CI script (golangci-lint run --no-config --disable-all --enable gosimple --enable errcheck --enable govet --enable unused --enable goconst --timeout 3m) does not show me any problems with original code.

This is no longer highlighted after updating the linter, so I've reverted back to original.
Maybe they fixed the linter in the update.
But when I use the original

block, _ := pem.Decode(b) if block == nil { logger.Fatal("No PEM data found, failed to decode CA") } caCert, err = x509.ParseCertificate(block.Bytes) <--- IDE warns here about possible nil pointer dereference even after checking for nil above if err != nil { return tlsCert, caCert, errors.Wrap(errFailedCertDecode, err) }

@dborovcanin Should I leave this as it was(original code) or change to switch statement?

Let's keep it as it is.

tools/mqtt-bench/bench.go

twins/api/http/endpoint_twins_test.go

bootstrap/api/endpoint_test.go

bootstrap/redis/producer/streams_test.go

bootstrap/service.go

scripts/ci.sh

auth/service_test.go

auth/jwt/tokenizer.go

auth/postgres/groups.go

auth/service.go

auth/service_test.go

bootstrap/redis/producer/streams.go

dborovcanin · 2023-04-17T09:10:16Z

cmd/certs/main.go

@@ -169,13 +169,14 @@ func loadCertificates(conf config, logger mflog.Logger) (tls.Certificate, *x509.
 	}

 	block, _ := pem.Decode(b)
-	if block == nil {
+	switch block {


Can you, please, send me the exact golangci-lint command to test this? The command we have in CI script (golangci-lint run --no-config --disable-all --enable gosimple --enable errcheck --enable govet --enable unused --enable goconst --timeout 3m) does not show me any problems with original code.

readers/cassandra/messages.go

readers/mocks/messages.go

readers/timescale/messages.go

Signed-off-by: aryan <aryangodara03@gmail.com>

drasko

LGTM

dborovcanin changed the title ~~MF-1718 - GolangCI checks need to resolve~~ MF-1718 - Use static code analysis in CI Feb 22, 2023

AryanGodara force-pushed the golangci-resolve branch from 7d87065 to add3d9f Compare March 29, 2023 14:39

codeball-ai-writer bot added the codeball:needs-careful-review label Mar 29, 2023

AryanGodara commented Mar 31, 2023

View reviewed changes

auth/api/grpc/setup_test.go Show resolved Hide resolved

AryanGodara commented Mar 31, 2023

View reviewed changes

things/api/auth/grpc/setup_test.go Outdated Show resolved Hide resolved

AryanGodara marked this pull request as ready for review April 2, 2023 11:15

AryanGodara requested a review from a team as a code owner April 2, 2023 11:15

rodneyosodo requested changes Apr 2, 2023

View reviewed changes

AryanGodara force-pushed the golangci-resolve branch from ee40c35 to f156cb8 Compare April 3, 2023 07:14

dborovcanin requested changes Apr 3, 2023

View reviewed changes

AryanGodara force-pushed the golangci-resolve branch 3 times, most recently from fe294ae to 21c55cc Compare April 4, 2023 13:30

dborovcanin requested changes Apr 4, 2023

View reviewed changes

AryanGodara force-pushed the golangci-resolve branch 2 times, most recently from 6020ca8 to 9aded14 Compare April 5, 2023 21:34

rodneyosodo requested changes Apr 6, 2023

View reviewed changes

dborovcanin requested changes Apr 6, 2023

View reviewed changes

auth/api/grpc/setup_test.go Outdated Show resolved Hide resolved

dborovcanin requested changes Apr 7, 2023

View reviewed changes

AryanGodara force-pushed the golangci-resolve branch 2 times, most recently from 4169b81 to 2ecced5 Compare April 12, 2023 22:17

drasko reviewed Apr 12, 2023

View reviewed changes

tools/mqtt-bench/bench.go Outdated Show resolved Hide resolved

AryanGodara force-pushed the golangci-resolve branch from 2ecced5 to 1c6a2af Compare April 12, 2023 23:09

dborovcanin requested changes Apr 13, 2023

View reviewed changes

AryanGodara force-pushed the golangci-resolve branch from 1c6a2af to 10300e9 Compare April 14, 2023 20:02

dborovcanin requested changes Apr 17, 2023

View reviewed changes

AryanGodara force-pushed the golangci-resolve branch 2 times, most recently from e5e6e2d to 3f7eddd Compare April 18, 2023 20:44

AryanGodara added 26 commits April 21, 2023 22:26

things, twins, and logger lint fixed

3177d1a

Signed-off-by: aryan <aryangodara03@gmail.com>

all services updated, auth jwt not working, ineffectual assignment issue

24d8b6d

Signed-off-by: aryan <aryangodara03@gmail.com>

handle error from grpc server in endpointtest

3e10361

Signed-off-by: aryan <aryangodara03@gmail.com>

temp commit, auth/jwt needs to be resolved

6fa690a

Signed-off-by: aryan <aryangodara03@gmail.com>

revert back to jwt v4 temporarily

34a21ca

Signed-off-by: aryan <aryangodara03@gmail.com>

updated jwt tokenizer

bfc1dcc

Signed-off-by: aryan <aryangodara03@gmail.com>

resolve EOF error for httptest requests

21f797d

Signed-off-by: aryan <aryangodara03@gmail.com>

fix auth jwt, update to registeredclaims

f089e41

Signed-off-by: aryan <aryangodara03@gmail.com>

fix ineffective assignment, auth/api/grpc endpoint failing

6a48d7a

Signed-off-by: aryan <aryangodara03@gmail.com>

temp commit, remove later

b9a8ffe

Signed-off-by: aryan <aryangodara03@gmail.com>

fix grpc server setup

ea41d83

Signed-off-by: aryan <aryangodara03@gmail.com>

resolve golangci tests, remove debug statements

1709ad5

Signed-off-by: aryan <aryangodara03@gmail.com>

update golangci version and modify linters used

a12b21c

Signed-off-by: aryan <aryangodara03@gmail.com>

fix failing tests

1cd3baa

Signed-off-by: aryan <aryangodara03@gmail.com>

fix grpc server for setup tests

9b15e30

Signed-off-by: aryan <aryangodara03@gmail.com>

fix logging and errors inlined

c79f217

Signed-off-by: aryan <aryangodara03@gmail.com>

fix remarks, update grpc setup_test

84b982b

Signed-off-by: aryan <aryangodara03@gmail.com>

fix setup_test

b40e835

Signed-off-by: aryan <aryangodara03@gmail.com>

update setup_test grpc

27fd8c5

Signed-off-by: aryan <aryangodara03@gmail.com>

fix data race

90b7454

Signed-off-by: aryan <aryangodara03@gmail.com>

update setup_test grpc

bf0fb24

Signed-off-by: aryan <aryangodara03@gmail.com>

fix grpc setup down to single simple function

5ac6558

Signed-off-by: aryan <aryangodara03@gmail.com>

fix linting issues

e903ec3

Signed-off-by: aryan <aryangodara03@gmail.com>

resolve pr comments

ad060b6

Signed-off-by: aryan <aryangodara03@gmail.com>

fix tests, handle returned errors, go mod tidy vendor

51bd182

Signed-off-by: aryan <aryangodara03@gmail.com>

fix errors from new linters

d121bd0

Signed-off-by: aryan <aryangodara03@gmail.com>

AryanGodara force-pushed the golangci-resolve branch from e8b1078 to d121bd0 Compare April 21, 2023 16:57

AryanGodara requested a review from a team April 22, 2023 09:28

drasko approved these changes Apr 22, 2023

View reviewed changes

drasko merged commit 54c7518 into absmach:master Apr 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

MF-1718 - Use static code analysis in CI #1729

MF-1718 - Use static code analysis in CI #1729

AryanGodara commented Feb 22, 2023

codecov-commenter commented Apr 1, 2023 •

edited

Loading

dborovcanin left a comment

AryanGodara commented Apr 3, 2023 •

edited

Loading

dborovcanin Apr 7, 2023

AryanGodara Apr 7, 2023

dborovcanin Apr 17, 2023

AryanGodara Apr 18, 2023

AryanGodara Apr 18, 2023

dborovcanin Apr 19, 2023

dborovcanin Apr 17, 2023

drasko left a comment

MF-1718 - Use static code analysis in CI #1729

MF-1718 - Use static code analysis in CI #1729

Conversation

AryanGodara commented Feb 22, 2023

What does this do?

Which issue(s) does this PR fix/relate to?

List any changes that modify/break current functionality

Have you included tests for your changes?

Did you document any new/modified functionality?

Notes

codecov-commenter commented Apr 1, 2023 • edited Loading

Codecov Report

dborovcanin left a comment

Choose a reason for hiding this comment

AryanGodara commented Apr 3, 2023 • edited Loading

dborovcanin Apr 7, 2023

Choose a reason for hiding this comment

AryanGodara Apr 7, 2023

Choose a reason for hiding this comment

dborovcanin Apr 17, 2023

Choose a reason for hiding this comment

AryanGodara Apr 18, 2023

Choose a reason for hiding this comment

AryanGodara Apr 18, 2023

Choose a reason for hiding this comment

dborovcanin Apr 19, 2023

Choose a reason for hiding this comment

dborovcanin Apr 17, 2023

Choose a reason for hiding this comment

drasko left a comment

Choose a reason for hiding this comment

codecov-commenter commented Apr 1, 2023 •

edited

Loading

AryanGodara commented Apr 3, 2023 •

edited

Loading