Open
Conversation
Benchmark ResultBenchmark diff with base branchBenchmark result |
tigrannajaryan
force-pushed
the
tigran/add-test-long-stream
branch
3 times, most recently
from
3f59e6d to
e27eab3
Compare
tigrannajaryan
force-pushed
the
tigran/add-test-long-stream
branch
from
e27eab3 to
54e086c
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds a new long-running test Test{{.StructName}}WriteReadLong to test writer/reader functionality with a much larger number of records (up to 500,000 in slow mode vs 1,000 in existing tests). The PR also refactors MemReaderWriter into an exported type in a separate file for broader use.
Changes:
- Added new
Test{{.StructName}}WriteReadLongtest function to the template that generates approximately 10,000-100,000 records (controlled by STEF_ENABLE_SLOW_TESTS environment variable) - Moved and exported
MemReaderWriterfromframe_test.goto a new public filememreaderwriter.gofor reuse in generated tests - Removed unnecessary
require.NotNilassertions forreader.Recordsince it's always initialized - Fixed fuzz tests to use proper
CopyFrompattern instead of mutating writer.Record directly - Added documentation clarifying that
mutateRandomleaves modified flags in an undefined state
Reviewed changes
Copilot reviewed 37 out of 37 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| stefc/templates/go/writer_test.go.tmpl | Template file: adds new long-running test, removes NotNil check, fixes fuzz test to use CopyFrom pattern |
| stefc/templates/go/struct.go.tmpl | Adds documentation comment about undefined state of modified flags after mutateRandom |
| go/pkg/memreaderwriter.go | New file: exports MemReaderWriter for use in generated tests |
| go/pkg/frame_test.go | Updates to use exported MemReaderWriter (but has critical bugs due to unexported field access) |
| go/otel/otelstef/*.go | Generated files: adds mutateRandom documentation comments |
| go/otel/otelstef/*writer_test.go | Generated test files: adds new long-running tests and fixes fuzz tests |
| examples/*/*writer_test.go | Generated test files: adds new long-running tests and fixes fuzz tests |
| examples//.go | Generated files: adds mutateRandom documentation comments |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
tigrannajaryan
force-pushed
the
tigran/add-test-long-stream
branch
3 times, most recently
from
564e535 to
c8558b4
Compare
The new test uses longer test sequences. The existing Test{{.StructName}}WriteRead
uses short sequences and does not mutate the records enough to test all
interesting codepaths. In the past we had bugs that were only visible when large
number of record varieties were generated. The new test generates approximately
500000 records in the slow mode, compared to only 1000 records generated by
existing Test{{.StructName}}WriteRead.
tigrannajaryan
force-pushed
the
tigran/add-test-long-stream
branch
from
c8558b4 to
a489b32
Compare
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
The new test uses longer test sequences. The existing Test{{.StructName}}WriteRead uses short sequences and does not mutate the records enough to test all interesting codepaths. In the past we had bugs that were only visible when large number of record varieties were generated. The new test generates approximately 500000 records in the slow mode, compared to only 1000 records generated by existing Test{{.StructName}}WriteRead.