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

[chore][pkg/stanaza] Fix and strengthen test case #28228

Merged

Conversation

djaglowski
Copy link
Member

@djaglowski djaglowski commented Oct 23, 2023

Follows: #28206

This PR fixes a setup problem with a particular test and further enhances it to make it more robust.

The problem was that it restarts a single operator, which is not strictly supported behavior. Instead, a new operator should be created from the same config.

The test involves moving a file and then validating that a log written to the original file location is read correctly. This enhances the test to also validate that additional logs written to the original (moved) file will be read as well.

@djaglowski djaglowski merged commit ac5407c into open-telemetry:main Oct 23, 2023
85 checks passed
@djaglowski djaglowski deleted the pkg-staza-fileconsumer-pollstop branch October 23, 2023 21:04
@github-actions github-actions bot added this to the next release milestone Oct 23, 2023
djaglowski added a commit that referenced this pull request Oct 24, 2023
Follows
#28228

This normalizes calls to `Start` and `Stop` across the test suite. 

In some cases, `poll` is called directly in order to trigger behavior
independently of timing. However, we should _either_ use `poll`
directly, or use both `Start` and `Stop` exactly once. In the future, I
expect `poll` will be exported and tested directly as part of an
internal package.
sigilioso pushed a commit to carlossscastro/opentelemetry-collector-contrib that referenced this pull request Oct 27, 2023
Follows:
open-telemetry#28206

This PR fixes a setup problem with a particular test and further
enhances it to make it more robust.

The problem was that it restarts a single operator, which is not
strictly supported behavior. Instead, a new operator should be created
from the same config.

The test involves moving a file and then validating that a log written
to the original file location is read correctly. This enhances the test
to also validate that additional logs written to the original (moved)
file will be read as well.
sigilioso pushed a commit to carlossscastro/opentelemetry-collector-contrib that referenced this pull request Oct 27, 2023
…-telemetry#28294)

Follows
open-telemetry#28228

This normalizes calls to `Start` and `Stop` across the test suite. 

In some cases, `poll` is called directly in order to trigger behavior
independently of timing. However, we should _either_ use `poll`
directly, or use both `Start` and `Stop` exactly once. In the future, I
expect `poll` will be exported and tested directly as part of an
internal package.
jmsnll pushed a commit to jmsnll/opentelemetry-collector-contrib that referenced this pull request Nov 12, 2023
Follows:
open-telemetry#28206

This PR fixes a setup problem with a particular test and further
enhances it to make it more robust.

The problem was that it restarts a single operator, which is not
strictly supported behavior. Instead, a new operator should be created
from the same config.

The test involves moving a file and then validating that a log written
to the original file location is read correctly. This enhances the test
to also validate that additional logs written to the original (moved)
file will be read as well.
jmsnll pushed a commit to jmsnll/opentelemetry-collector-contrib that referenced this pull request Nov 12, 2023
…-telemetry#28294)

Follows
open-telemetry#28228

This normalizes calls to `Start` and `Stop` across the test suite. 

In some cases, `poll` is called directly in order to trigger behavior
independently of timing. However, we should _either_ use `poll`
directly, or use both `Start` and `Stop` exactly once. In the future, I
expect `poll` will be exported and tested directly as part of an
internal package.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants