Skip to content

fix: fix data race in TestLabelsAggregation tests #12578

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

Merged
merged 2 commits into from
Mar 13, 2024

Conversation

dannykopping
Copy link
Contributor

Signed-off-by: Danny Kopping <danny@coder.com>
@dannykopping dannykopping changed the title fix: data race in TestLabelsAggregation tests fix: fix data race in TestLabelsAggregation tests Mar 13, 2024
@dannykopping dannykopping requested a review from johnstcn March 13, 2024 10:36
Comment on lines 208 to 214
var expectedLabels []*agentproto.Stats_Metric_Label
for _, l := range e.Labels {
expectedLabels = append(expectedLabels, &agentproto.Stats_Metric_Label{
Name: l.Name,
Value: l.Value,
})
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not copy(expectedLabels, e.Labels)?

https://go.dev/wiki/SliceTricks#copy

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call! I tried it before but it wasn't working, but now I realise I needed to size the dst slice first 🤦
Fixed in 9113c015e

Signed-off-by: Danny Kopping <danny@coder.com>
@dannykopping dannykopping merged commit da54c8a into coder:main Mar 13, 2024
@dannykopping dannykopping deleted the dk/test-race branch March 13, 2024 11:47
@github-actions github-actions bot locked and limited conversation to collaborators Mar 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants