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/stanza] Add detailed split func test helper #26717

Merged
merged 2 commits into from
Sep 26, 2023

Conversation

djaglowski
Copy link
Member

@djaglowski djaglowski commented Sep 16, 2023

This PR migrates all tests in the split package to the new framework.

Notable changes:

  • No longer a need to have special non-standard tests. These are either redundant, or were absorbed into the standard pattern.
    • Flush at EOF tests moved into standard pattern.
    • Encoded newline tests moved into standard pattern.
    • Removes tests relating to flushing, as these are a concern of the flush package now.
    • Removes tests relating to trimming, as these are a concern of the trim package now.
    • Removes tests for buffer overflow, as these were caused by using bufio.Scanner.

@djaglowski djaglowski force-pushed the pkg-stanza-split-test branch 2 times, most recently from 65f2ab4 to 46b4fbf Compare September 20, 2023 22:32
@djaglowski djaglowski marked this pull request as ready for review September 20, 2023 22:49
@djaglowski djaglowski requested review from a team and Aneurysm9 September 20, 2023 22:49
djaglowski added a commit that referenced this pull request Sep 22, 2023
Subset of #26717

Our current split func tests depend on a `bufio.Scanner`, which has the
advantage of being somewhat similar to our typical usage of the split
func. However, this does not allow us to validate some detailed
expectations of the split funcs, such as what they report for `advance`.

This is a new framework for testing split funcs. It's attempts to
thoroughly exercise the "read more" behavior of every function. It has
capabilities for inserting sleeps in between steps, and always validates
that each test ends with the split func returning a "read more",
indicating that it can find no more tokens in the input.
@djaglowski djaglowski force-pushed the pkg-stanza-split-test branch from c0d3257 to 3fb7794 Compare September 22, 2023 21:42
@djaglowski
Copy link
Member Author

@open-telemetry/collector-contrib-approvers, this is waiting for a review.

@djaglowski djaglowski merged commit 207b6c1 into open-telemetry:main Sep 26, 2023
@djaglowski djaglowski deleted the pkg-stanza-split-test branch September 26, 2023 21:42
@github-actions github-actions bot added this to the next release milestone Sep 26, 2023
jmsnll pushed a commit to jmsnll/opentelemetry-collector-contrib that referenced this pull request Nov 12, 2023
…try#27074)

Subset of open-telemetry#26717

Our current split func tests depend on a `bufio.Scanner`, which has the
advantage of being somewhat similar to our typical usage of the split
func. However, this does not allow us to validate some detailed
expectations of the split funcs, such as what they report for `advance`.

This is a new framework for testing split funcs. It's attempts to
thoroughly exercise the "read more" behavior of every function. It has
capabilities for inserting sleeps in between steps, and always validates
that each test ends with the split func returning a "read more",
indicating that it can find no more tokens in the input.
jmsnll pushed a commit to jmsnll/opentelemetry-collector-contrib that referenced this pull request Nov 12, 2023
…ry#26717)

This PR migrates all tests in the `split` package to the new framework.

Notable changes:
- No longer a need to have special non-standard tests. These are either
redundant, or were absorbed into the standard pattern.
  - Flush at EOF tests moved into standard pattern.
  - Encoded newline tests moved into standard pattern.
- Removes tests relating to flushing, as these are a concern of the
`flush` package now.
- Removes tests relating to trimming, as these are a concern of the
`trim` package now.
- Removes tests for buffer overflow, as these were caused by using
`bufio.Scanner`.
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