-
Notifications
You must be signed in to change notification settings - Fork 1.8k
[componenttest] Remove GetFactory from nopHost
#13577
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
[componenttest] Remove GetFactory from nopHost
#13577
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #13577 +/- ##
==========================================
- Coverage 92.24% 92.24% -0.01%
==========================================
Files 614 614
Lines 33686 33684 -2
==========================================
- Hits 31075 31071 -4
- Misses 2076 2077 +1
- Partials 535 536 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like contrib needs fixing after this PR https://github.com/open-telemetry/opentelemetry-collector/actions/runs/16781207495/job/47520490032?pr=13577
|
I've been looking into that, unfortunately it's more complicated than just fixing up a test: it's a generated test, so either we have to disable lifecycle tests on the receivercreator, create a Right now I'm leaning toward disabling the generated tests and writing more comprehensive tests to validate the receiver's startup behavior. The receiver is unique enough that I think it warrants having its own tests. |
|
@evan-bradley I think we should change mdatagen to call with a host that implements the |
|
@bogdandrutu that's okay with me. I've implemented it here: #13589. I think that unless we provide options to generate a test matrix for whether calls to |
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description Use a custom noop host implementation that implements all non-deprecated, publicly-accessible interfaces implemented by the Collector service. I didn't implement the `ExposeExporters` interface since it is deprecated. Related to #13577.
Description
The
GetFactorymethod was moved fromcomponent.Hosttohostcapabilities.ComponentFactory. From what I can tell, this method is not used on the no-op host anywhere in core, and introduces an implicit loop in the dependency graph, so we should remove it beforecomponenttestis made stable.Link to tracking issue
Works toward #13490