Skip to content
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

[exporter/datasetexporter]: Fix failing test on windows #22041

Conversation

martin-majlis-s1
Copy link
Contributor

@martin-majlis-s1 martin-majlis-s1 commented May 17, 2023

Description: In the PR #21815 I have added code for dataset exporter. However test for windows - https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/5002859274/jobs/8963342956 - is failing. So let's fix it.

Link to tracking Issue: #22039, #20660

Testing:

The issue is caused by the github.com/stretchr/testify/suite package. Once I have removed it started to work.

@martin-majlis-s1
Copy link
Contributor Author

Library httptest is recommended and it's used everywhere. As well as problematic function - httptest.NewServer - https://github.com/search?q=repo%3Aopen-telemetry%2Fopentelemetry-collector-contrib%20httptest.NewServer&type=code

@martin-majlis-s1
Copy link
Contributor Author

@mx-psi : This is the PR, where I am trying to figure out, why is it failing.

@mx-psi mx-psi added the Run Windows Enable running windows test on a PR label May 17, 2023
@mx-psi
Copy link
Member

mx-psi commented May 17, 2023

@martin-majlis-s1 I added the 'Run Windows' label. Future commits to the PR's branch will run the test suite on Windows.

@martin-majlis-s1
Copy link
Contributor Author

Message - " socket: The requested service provider could not be loaded or initialized." - is universal message in windows.

This is happening when the application does not have permissions - https://travis-ci.community/t/socket-the-requested-service-provider-could-not-be-loaded-or-initialized/1127 - but it's really strange, since it is used by everybody - #22041 (comment)

@mx-psi mx-psi added the Skip Changelog PRs that do not require a CHANGELOG.md entry label May 17, 2023
@martin-majlis-s1
Copy link
Contributor Author

@martin-majlis-s1
Copy link
Contributor Author

@mx-psi : I can remove/skip that test and then I can try to figure out, why is that test failing.

Copy link
Member

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

Please fix lint

exporter/datasetexporter/logs_exporter_test.go Outdated Show resolved Hide resolved
@martin-majlis-s1
Copy link
Contributor Author

martin-majlis-s1 commented May 18, 2023

My latest discovery is that it depends on the order. If the logs_exporter_test.go runs first, then it succeeds. When it's last, then it fails. Now I am trying to figure out why is this happening.

Succeeds:

go test -race -parallel 4 --tags="" ./logs_exporter_test.go ./datasetexporter_test.go logs_exporter.go  config.go datasetexporter.go traces_exporter.go factory.go factory_test.go traces_exporter_test.go config_test.go doc.go

Fails:

go test -race -parallel 4 --tags="" ./datasetexporter_test.go logs_exporter.go  config.go datasetexporter.go traces_exporter.go factory.go factory_test.go traces_exporter_test.go config_test.go doc.go ./logs_exporter_test.go 
Screenshot 2023-05-18 at 13 40 49

EDIT: This comment is little bit misleading. In this code I have also removed the suite package. When I have removed the suite package back, it was failing again in any order.

@mx-psi mx-psi requested review from songy23 and MovieStoreGuy May 18, 2023 15:05
@martin-majlis-s1
Copy link
Contributor Author

So the final finding is that it's cause by the library - https://pkg.go.dev/github.com/stretchr/testify/suite. Once I have removed it - it started to work. It's no clear to me why.

Copy link
Member

@songy23 songy23 left a comment

Choose a reason for hiding this comment

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

LGTM

@TylerHelmuth TylerHelmuth merged commit a7aedb2 into open-telemetry:main May 18, 2023
@github-actions github-actions bot added this to the next release milestone May 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
exporter/dataset Run Windows Enable running windows test on a PR Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants