-
Notifications
You must be signed in to change notification settings - Fork 258
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Go test race conditions on ./pkg/endpoints
#73
Comments
I'll take a look at this. https://github.com/tektoncd/dashboard/blob/master/test/presubmit-tests.sh#L132 will need to be changed once resolved, and the reproduce is noddy, just clone this:
So surely there's a read on the socket at the same time as we write, here's a snippet from my stacktrace for the first problem:
|
Let's see if I can assign OK now, Vincent wrote most of this code so we're going to buddy up (and if it doesn't work I get to search the repos for me saying this and retrospectively assign him) /assign @vtereso |
@a-roberts: GitHub didn't allow me to assign the following users: vtereso. Note that only tektoncd members and repo collaborators can be assigned. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
/assign vtereso |
@vtereso produced a fix for this y'day and I've tidied things up, added more comments, merged with master and resolved an import problem. I've also added the -race check back in. https://github.com/tektoncd/dashboard/compare/master...a-roberts:websocketRacev2?expand=1 here's the code on my branch and here's the original: https://github.com/vtereso/dashboard/tree/websocketRace. I'm going to wait until Vincent is back online and then he can replace the contents of his branch with mine, squash commits and get the PR in so he can get the credit. Thanks @vtereso! |
#94 is in, we can close this one ! |
@vdemeester: Closing this issue. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
There is some data race (caught by
go test -race
) on./pkg/endpoints
.TestLogWebsocket
TestPipelineRunWebsocket
I disabled race detection on the CI for now (to bootstrap it), but this need to be fixed (so that we can re-enable race detections 👼 )
cc @a-roberts
See https://tekton-releases.appspot.com/build/tekton-prow/pr-logs/pull/tektoncd_dashboard/72/pull-tekton-dashboard-tests/1120985522127245312/
The text was updated successfully, but these errors were encountered: