fix: store tuples being required #541
Conversation
WalkthroughThe changes update the logic for loading tuples in the test store: tuple files are now only loaded if specified, and the absence of tuple files no longer results in an error. The related test is adjusted to expect zero tuples instead of an error when no tuple files are provided. Changes
Sequence Diagram(s)sequenceDiagram
participant TestRunner
participant StoreData
participant FileSystem
TestRunner->>StoreData: LoadTuples()
alt TupleFile is specified
StoreData->>FileSystem: loadAndAddTuplesFromFile(TupleFile)
FileSystem-->>StoreData: Tuples
end
alt TupleFiles is non-empty
loop for each file in TupleFiles
StoreData->>FileSystem: loadAndAddTuplesFromFile(file)
FileSystem-->>StoreData: Tuples
end
end
StoreData->>StoreData: loadTestTuples()
StoreData-->>TestRunner: Loaded Tuples
Assessment against linked issues
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🧰 Additional context used🧬 Code Graph Analysis (1)internal/storetest/storedata_test.go (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
🔇 Additional comments (2)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
53a9078 to
30a56cd
Compare
30a56cd to
5b188a3
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR relaxes the requirement that at least one tuples source must be provided to StoreData.LoadTuples, making tuple inputs fully optional and updating tests accordingly.
- Removed the error when no tuple files or inline tuples are specified
- Guard tuple-loading calls behind presence checks for
TupleFileandTupleFiles - Updated the test case to expect zero tuples instead of an error when no sources are given
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| internal/storetest/storedata.go | Removed mandatory-tuple error, added presence checks for loading |
| internal/storetest/storedata_test.go | Changed test to assert zero tuples instead of an error |
Comments suppressed due to low confidence (3)
internal/storetest/storedata_test.go:50
- Ensure the test runner logic is updated to use
expectTuples—assert thaterris nil whenexpectTuplesis defined and verify the loaded tuples count matches the expected value.
expectTuples: 0,
internal/storetest/storedata.go:133
- [nitpick] Consider validating that
TupleFileandTupleFilesaren’t both set at once to avoid duplicate or conflicting loads. For example, return an error if both are provided.
if storeData.TupleFile != "" {
internal/storetest/storedata.go:130
- Update the function’s doc comment to reflect that tuples are now optional and no longer required, aligning the documentation with the new behavior.
addTuples(storeData.Tuples)
Description
This change allows users to:
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
closes #539
Review Checklist
mainSummary by CodeRabbit
Bug Fixes
Tests