-
Notifications
You must be signed in to change notification settings - Fork 4
Abhishek/java seed impl #303
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
base: main
Are you sure you want to change the base?
Conversation
…ing test cases and rerun
…ing test cases and rerun
Benchmark ResultBenchmark diff with base branchBenchmark result |
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.
Pull request overview
This PR implements seed-based regression testing for Java-generated schema tests. When a randomized test fails, the failing seed is automatically stored in a language-specific seed file and replayed in subsequent test runs to prevent regressions.
Key changes:
- Refactored Java test template to extract test logic into a separate seed-based method
- Added automatic seed file management with load/save functionality
- Created Java-specific seed storage directory with documentation
- Updated protobuf-generated Go files (unrelated to main feature)
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| stefc/templates/java/writerTest.java.tmpl | Refactored test template to support seed loading/saving with file I/O operations |
| stefc/generator/testdata/seeds/java/com.example.gentest.optional_reset_fail_Struct1_seeds.txt | Sample seed file with two previously-failing test seeds |
| stefc/generator/testdata/seeds/java/com.example.gentest.json_like_Record_seeds.txt | Sample seed file with one previously-failing test seed |
| stefc/generator/testdata/seeds/java/README.md | Documentation explaining the seed storage mechanism and naming conventions |
| go/grpc/stef_proto/destination_grpc.pb.go | Regenerated protobuf gRPC bindings with updated protoc tooling versions |
| go/grpc/stef_proto/destination.pb.go | Regenerated protobuf message definitions with updated protoc tooling versions |
| examples/jsonl/internal/jsonpb/jsonl.pb.go | Regenerated protobuf message definitions with updated protoc tooling versions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } catch (Exception e) { | ||
| fail("seed " + seed1 + " optIdx " + optIdx + ": " + e.getMessage()); | ||
| } catch (Throwable t) { | ||
| System.out.printf("Test failed with seed %d optIdx %d: %s%n", seed, optIdx, t); |
Copilot
AI
Dec 19, 2025
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.
The test method catches all Throwable exceptions but continues execution and only returns false instead of propagating the exception. This means that if an error occurs during test setup (e.g., writer creation fails), the test will silently fail for that optIdx but continue with other iterations. Consider propagating exceptions or at least logging them with the full stack trace. The current implementation only prints the message with toString(), which may not provide enough diagnostic information.
| System.out.printf("Test failed with seed %d optIdx %d: %s%n", seed, optIdx, t); | |
| System.out.printf("Test failed with seed %d optIdx %d: %s%n", seed, optIdx, t); | |
| t.printStackTrace(System.out); |
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.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| Path seedFilePath = Paths.get("../stefc/generator/testdata/seeds/java/{{ .PackageName }}_{{.StructName}}_seeds.txt"); | ||
|
|
||
| if (Files.exists(seedFilePath)) { | ||
| for (String line : Files.readAllLines(seedFilePath)) { |
Copilot
AI
Dec 19, 2025
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.
The Files.readAllLines call can throw IOException if the file cannot be read. This exception should be explicitly handled to provide better error messages if the seed file exists but cannot be read due to permission issues or other I/O problems.
Co-authored-by: Copilot <[email protected]>
tigrannajaryan
left a comment
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.
Great job with the PR, just one minor comment. Once you remove the unrelated file changes, we can merge. I have also run it and verified locally, everything appears to work correctly.
| @@ -1,7 +1,7 @@ | |||
| // Code generated by protoc-gen-go. DO NOT EDIT. | |||
| // versions: | |||
| // protoc-gen-go v1.26.0 | |||
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.
Please do not include unrelated changes in the PR. This file and files in stef_proto don't belong in this PR.
Looks like you are using a different version of protoc and running make all resulted in the changes. You can either restore original files or install the same version of protoc that was used to generate the files last time so that running make all doesn't change the generated files.
In the future we should probably make installing specific protoc version part of https://github.com/splunk/stef/blob/main/CONTRIBUTING.md
It can be done in a future PR.
|
Please update PR title to reflect the change better. |
Summary: Java Seed-Based Regression Testing Implementation
What We Built
How It Works
On each test run:
When a test fails: