test(sdk): Use -race Flag when testing #1026
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lenny-goodell
requested changes
Jan 25, 2022
Makefile
Outdated
| @@ -34,7 +34,7 @@ test-template: | |||
| make -C ./app-service-template test | |||
|
|
|||
| test-sdk: | |||
| $(GO) test ./... -coverprofile=coverage.out ./... | |||
| $(GO) test ./... -coverprofile=coverage.out ./... -race | |||
There was a problem hiding this comment.
This looks strange... with the double ./... that was there before. In edgex-go it looks like this:
GOTESTFLAGS?=-race
.
.
.
GO111MODULE=on go test $(GOTESTFLAGS) -coverprofile=coverage.out ./...
Please use the edgex-go way of doing it.
There was a problem hiding this comment.
Also this should be added to the Makefile for the app-service-template.
There was a problem hiding this comment.
Ah I wondered what that was about @lenny-intel - should be good now thanks
AlexCuse
force-pushed
the
fix/test-race
branch
from
a101583 to
c2abb10
Compare
lenny-goodell
requested changes
Jan 25, 2022
app-service-template/Makefile
Outdated
| @@ -59,7 +59,7 @@ docker: | |||
| # The test-attribution-txt.sh scripts are required for upstreaming to EdgeX Foundry. | |||
| # TODO: Remove bin folder and reference to script below if NOT upstreaming to EdgeX Foundry. | |||
| test: | |||
| $(GO) test -coverprofile=coverage.out ./... | |||
| $(GO) test -coverprofile=coverage.out ./... --race | |||
There was a problem hiding this comment.
Still not consistent with edgex-go. Use variable and position of opinion. Please update to use:
GOTESTFLAGS?=-race
$(GO) test $(GOTESTFLAGS) -coverprofile=coverage.out ./...
There was a problem hiding this comment.
Includes naive fix for one race detected. Fixes #209 Signed-off-by: Alex Ullrich <[email protected]>
AlexCuse
force-pushed
the
fix/test-race
branch
from
c2abb10 to
c046d6b
Compare
FelixTing
pushed a commit
to FelixTing/app-functions-sdk-go
that referenced
this pull request
Mar 3, 2022
Includes naive fix for one race detected. Fixes edgexfoundry#209 Signed-off-by: Alex Ullrich <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Includes naive fix for one race detected.
Fixes #209
Signed-off-by: Alex Ullrich [email protected]
If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/app-functions-sdk-go/blob/main/.github/CONTRIBUTING.md
PR Checklist
Please check if your PR fulfills the following requirements:
BREAKING CHANGE:describing the break)no docs change
Testing Instructions
New Dependency Instructions (If applicable)