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.

Sign up for GitHub

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

Jump to bottom

[test][receiver/awss3] Test_opampNotifier_SendStatus_(MessagePending|MaxRetries) are broken on Windows #35704

Closed
pjanotti opened this issue Oct 8, 2024 · 2 comments · Fixed by #35714
Closed

[test][receiver/awss3] Test_opampNotifier_SendStatus_(MessagePending|MaxRetries) are broken on Windows #35704

pjanotti opened this issue Oct 8, 2024 · 2 comments · Fixed by #35714
Labels
os:windows receiver/awss3

Comments

@pjanotti
Copy link
Contributor

pjanotti commented Oct 8, 2024

Component(s)

receiver/awss3

Describe the issue you're reporting

Usage of completionTime.After(now) is not reliable on Windows since the default clock on the platform is a very low resolution, see #32715 (comment)

https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/11239757804/job/31247553663#step:8:250

=== FAIL: . Test_opampNotifier_SendStatus_MessagePending (0.00s)
    notifications_test.go:237: 
        	Error Trace:	D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/receiver/awss3receiver/notifications_test.go:237
        	Error:      	Should be true
        	Test:       	Test_opampNotifier_SendStatus_MessagePending

=== FAIL: . Test_opampNotifier_SendStatus_MaxRetries (0.00s)
    notifications_test.go:285: 
        	Error Trace:	D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/receiver/awss3receiver/notifications_test.go:285
        	Error:      	Should be true
        	Test:       	Test_opampNotifier_SendStatus_MaxRetries

=== FAIL: . Test_opampNotifier_SendStatus_MessagePending (re-run 1) (0.00s)
    notifications_test.go:237: 
        	Error Trace:	D:/a/opentelemetry-collector-contrib/opentelemetry-collector-contrib/receiver/awss3receiver/notifications_test.go:237
        	Error:      	Should be true
        	Test:       	Test_opampNotifier_SendStatus_MessagePending

DONE 2 runs, 79 tests, 3 failures in 50.242s
@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 8, 2024

Pinging code owners for receiver/awss3: @atoulme @adcharre. See Adding Labels via Comments if you do not have permissions to add labels yourself.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Oct 8, 2024

Pinging code owners:

See Adding Labels via Comments if you do not have permissions to add labels yourself.

@adcharre adcharre mentioned this issue Oct 9, 2024
Merged
@atoulme atoulme removed the needs triage New item requiring triage label Oct 12, 2024
@github-actions github-actions bot mentioned this issue Oct 15, 2024
Closed
@mx-psi mx-psi closed this as completed in cb3e537 Oct 15, 2024
sbylica-splunk pushed a commit to sbylica-splunk/opentelemetry-collector-contrib that referenced this issue Dec 17, 2024
@adcharre @sbylica-splunk
awss3receiver: Fix notifications test which fail when run on Windows (o…
094d25e 
…pen-telemetry#35714)

#### Description
Remove the use of time to detect the order of completion and replace
with channels.

#### Link to tracking issue
Fixes open-telemetry#35704

#### Testing
Tested that the new method detects failure to wait on the pending
channel by removing the code in SendStatus() that receives from the
channel and confirming that the test fails.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
os:windows receiver/awss3
Projects
None yet
Milestone
No milestone
2 participants
@atoulme @pjanotti