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

[receiver/hostmetrics] Unit tests don't match implementation for darwin #19141

Closed
antonblock opened this issue Feb 28, 2023 · 12 comments
Closed
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed priority:p3 Lowest receiver/hostmetrics

Comments

@antonblock
Copy link
Contributor

Component(s)

receiver/hostmetrics

What happened?

Description

Since #18723, a subset of process metrics is supported on Darwin. When I run unit tests locally on a Mac, the processscraper/TestCreateResourceMetricsScraper test fails because the method no longer returns an error. Additionally, the skipTestOnUnsupportedOS helper disables tests for metrics that are now supported on Darwin.

Steps to Reproduce

On Darwin, run unit tests from the github.com/open-telemetry/opentelemetry-collector-contrib/receiver/hostmetricsreceiver package.

Expected Result

Tests pass, and all metrics supported on Darwin are tested.

Actual Result

The TestCreateResourceMetricsScraper test fails, most process_scraper methods are not tested.

Collector version

11f8ed1

Environment information

Environment

OS: macOS 13.2.1
Compiler(if manually compiled): go 1.20.1

OpenTelemetry Collector configuration

No response

Log output

--- FAIL: TestCreateResourceMetricsScraper (0.00s)
    factory_test.go:42:
        	Error Trace:	/Users/nico/contrib/receiver/hostmetricsreceiver/internal/scraper/processscraper/factory_test.go:42
        	Error:      	An error is expected but got nil.
        	Test:       	TestCreateResourceMetricsScraper
    factory_test.go:43:
        	Error Trace:	/Users/nico/contrib/receiver/hostmetricsreceiver/internal/scraper/processscraper/factory_test.go:43
        	Error:      	Expected nil, but got: &scraperhelper.baseScraper{StartFunc:(component.StartFunc)(0x1052db640), ShutdownFunc:(component.ShutdownFunc)(nil), ScrapeFunc:(scraperhelper.ScrapeFunc)(0x1052db5c0), id:component.ID{typeVal:"process", nameVal:""}}
        	Test:       	TestCreateResourceMetricsScraper
FAIL

Additional context

No response

@antonblock antonblock added bug Something isn't working needs triage New item requiring triage labels Feb 28, 2023
@github-actions
Copy link
Contributor

Pinging code owners:

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

@dmitryax dmitryax added help wanted Extra attention is needed priority:p3 Lowest and removed needs triage New item requiring triage labels Mar 1, 2023
@dmitryax
Copy link
Member

dmitryax commented Mar 1, 2023

@8naama, do you have a chance to submit a PR to update the tests?

@andrzej-stencel
Copy link
Member

I can pick this up if @8naama does not respond.

@dmitryax
Copy link
Member

Thanks, @astencel-sumo. Assigned to you

@8naama
Copy link
Contributor

8naama commented Mar 25, 2023

hey guys @dmitryax @astencel-sumo, really sorry! I completely missed this!
Let me know if it's still relevant and I'll check

@andrzej-stencel
Copy link
Member

No worries @8naama, great to see you back! 😄 If you can contribute the fix, that would be great 👍

@andrzej-stencel andrzej-stencel removed their assignment Mar 27, 2023
@andrzej-stencel
Copy link
Member

I've unassigned my self from this issue, @8naama should probably be assigned if I'm understanding correctly, is this right @8naama?

@8naama
Copy link
Contributor

8naama commented Mar 27, 2023

@astencel-sumo yes you can assign to me 🙏🏼

@dmitryax
Copy link
Member

Thanks, @8naama. Assigned to you

@8naama
Copy link
Contributor

8naama commented Apr 11, 2023

Solved in #20538

@andrzej-stencel
Copy link
Member

Thank you so much @8naama. Looking forward to your future contributions! 🙂

@8naama
Copy link
Contributor

8naama commented Apr 11, 2023

Thank you @astencel-sumo!! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed priority:p3 Lowest receiver/hostmetrics
Projects
None yet
Development

No branches or pull requests

4 participants