Ensure all exporters are added to lifecycle test #6933
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
In order to help test coverage up and ensuring that all components are tested, I have added what I could to the lifecycle tests for exporters while adding two additional helps within test util.
MovieStoreGuy
commented
Dec 22, 2021
| @@ -444,7 +443,7 @@ func CreateSentryExporter(config *Config, set component.ExporterCreateSettings) | |||
| allEventsFlushed := transport.Flush(ctx) | |||
|
|
|||
| if !allEventsFlushed { | |||
| log.Print("Could not flush all events, reached timeout") | |||
| set.Logger.Warn("Could not flush all events, reached timeout") | |||
There was a problem hiding this comment.
Updated this due to the amount of spam it produced throughout testing.
| @@ -60,8 +82,7 @@ func TestDefaultExporters(t *testing.T) { | |||
| exporter: "file", | |||
| getConfigFn: func() config.Exporter { | |||
| cfg := expFactories["file"].CreateDefaultConfig().(*fileexporter.Config) | |||
| f, err := ioutil.TempFile("", "otelcol_defaults_file_exporter_test*.tmp") | |||
| require.NoError(t, err) | |||
| f := testutil.NewTemporaryFile(t) | |||
There was a problem hiding this comment.
Moved to test helper so my /tmp would clear up after each test run
MovieStoreGuy
force-pushed
the
msg/make-components-testable
branch
from
600d53d
to
c51624c
Compare
MovieStoreGuy
force-pushed
the
msg/make-components-testable
branch
from
7dd56b2
to
1ace367
Compare
|
I did notice there is a few exports that haven't been added into components list as of yet, not sure if there are already issues to follow up on those. |
jpkrohling
approved these changes
Dec 27, 2021
| @@ -444,7 +443,7 @@ func CreateSentryExporter(config *Config, set component.ExporterCreateSettings) | |||
| allEventsFlushed := transport.Flush(ctx) | |||
|
|
|||
| if !allEventsFlushed { | |||
| log.Print("Could not flush all events, reached timeout") | |||
| set.Logger.Warn("Could not flush all events, reached timeout") | |||
| } | ||
|
|
||
| assert.Equal(t, len(tests)+25 /* not tested */, len(expFactories)) | ||
| assert.Equal(t, len(tests), len(expFactories), "All user configurable components must be added to the lifecycle test") |
There was a problem hiding this comment.
You can use assert.Len here.
There was a problem hiding this comment.
I'll add them in a follow up PR
|
I'll merge this PR as is, but if there are concerns about the sentry exporter, please open an issue to address them, @AbhiPrasad. |
animetauren
pushed a commit
to animetauren/opentelemetry-collector-contrib
that referenced
this pull request
Apr 4, 2023
…y#6933) * Update all modules to use v0.69.0 and 1.0.0-rc3 of collector modules
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description:
In order to help test coverage up and ensuring that all components are
tested, I have added what I could to the lifecycle tests for exporters
while adding two additional helps within test util.
Link to tracking Issue:
Yet to check if there was an open issue for this
Testing:
Tests were only added for validate the new test helpers, the rest were extending test suites that already exist.
Documentation:
The documentation had been updated in an earlier PR to include adding new components as part of these lifecycle tests.