test: composed contract with start date #667
Conversation
9a5e786 to
78aa227
Compare
WalkthroughThe pull request introduces several modifications across multiple files, primarily focusing on enhancing the testing framework and managing taxonomies within the codebase. Key changes include updates to the Changes
Assessment against linked issues
Possibly related PRs
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (3)
internal/contracts/tests/utils/procedure/execute.go (1)
137-159: LGTM: New function is well-implemented, with a minor suggestion.The
DescribeTaxonomiesfunction is well-structured and consistent with other functions in the file. It correctly handles errors and follows the established patterns for procedure execution.However, there's one minor point to consider:
The
Heightfield inTransactionDatais hardcoded to 0 (line 151). Consider parameterizing this value by adding aHeightfield to theDescribeTaxonomiesInputstruct, similar to other input structs in the file. This would make the function more flexible and consistent with the existing pattern.Here's a suggested change:
type DescribeTaxonomiesInput struct { Platform *kwilTesting.Platform DBID string LatestVersion bool + Height uint64 } // In the DescribeTaxonomies function: TransactionData: common.TransactionData{ Signer: input.Platform.Deployer, Caller: deployer.Address(), TxID: input.Platform.Txid(), - Height: 0, + Height: input.Height, },This change would align the
DescribeTaxonomiesfunction more closely with the other functions in the file.internal/contracts/composed_stream_template.kf (2)
812-813: Ensure consistent inclusion ofversionandstart_dateindescribe_taxonomiesIn the
describe_taxonomiesprocedure, the fieldsversionandstart_dateare added to the return table definition and the SELECT statements. Verify that these fields are consistently included in all parts of the procedure to prevent discrepancies between the defined return structure and the actual output.Also applies to: 823-824, 834-835
842-842: Implement disabling taxonomy bytaxonomy_idThere's a TODO comment indicating the need to disable a taxonomy by
taxonomy_id. Implementing this functionality would allow more granular control over individual taxonomies.Would you like assistance in drafting the implementation for this feature? I can help generate the necessary code or open a GitHub issue to track this task.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
go.sumis excluded by!**/*.sum
📒 Files selected for processing (7)
- Taskfile.yml (1 hunks)
- go.mod (1 hunks)
- internal/contracts/composed_stream_template.kf (7 hunks)
- internal/contracts/tests/composed_test.go (10 hunks)
- internal/contracts/tests/utils/procedure/execute.go (1 hunks)
- internal/contracts/tests/utils/setup/composed.go (5 hunks)
- scripts/download-binaries-dev.sh (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- go.mod
🧰 Additional context used
📓 Learnings (1)
internal/contracts/tests/composed_test.go (3)
Learnt from: outerlook PR: truflation/tsn#629 File: internal/contracts/tests/composed_test.go:447-501 Timestamp: 2024-09-24T19:06:30.810Z Learning: In the `internal/contracts/tests` package, test helper functions like `setTaxonomy` are used only within tests and may not require the same validation checks as production code.Learnt from: outerlook PR: truflation/tsn#629 File: internal/contracts/tests/composed_test.go:447-501 Timestamp: 2024-10-08T15:59:14.905Z Learning: In the `internal/contracts/tests` package, test helper functions like `setTaxonomy` are used only within tests and may not require the same validation checks as production code.Learnt from: outerlook PR: truflation/tsn#629 File: internal/contracts/tests/composed_test.go:447-501 Timestamp: 2024-10-08T15:59:14.952Z Learning: In the `internal/contracts/tests` package, test helper functions like `setTaxonomy` are used only within tests and may not require the same validation checks as production code.
🔇 Additional comments (11)
Taskfile.yml (1)
134-136: Verify test coverage after directory change.The change from
./internal/extensions/...to./internal/contracts/tests/...aligns with the PR objective of adding a test for setting a taxonomy with a start date. This reorganization of the test structure is a good practice for better code organization.However, please ensure that this change doesn't inadvertently exclude any important unit tests that were previously covered.
To verify the test coverage, please run the following commands:
Please review the output to ensure all necessary tests are included in the new directory structure.
✅ Verification successful
[/run_scripts]
Test coverage verified after directory change.
All unit tests have been successfully moved to
./internal/contracts/tests/, and the old./internal/extensions/directory has been removed. No necessary tests have been excluded from the new directory structure.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Compare the test files covered before and after the change # Test: List all test files in the old directory echo "Test files in ./internal/extensions/:" fd -e go -e test.go . ./internal/extensions # Test: List all test files in the new directory echo "Test files in ./internal/contracts/tests/:" fd -e go -e test.go . ./internal/contracts/tests # Test: Check if there are any test files left in the old directory echo "Checking for any remaining test files in ./internal/extensions/:" fd -e go -e test.go . ./internal/extensionsLength of output: 1405
Script:
#!/bin/bash # Description: Verify that all test files have been moved to ./internal/contracts/tests/ and no tests are missing. # Search for test files outside the new directory echo "Searching for test files outside ./internal/contracts/tests/:" fd -e go -e test.go . --exclude ./internal/contracts/tests/ # Verify that the old directory ./internal/extensions/ does not exist echo "Checking if ./internal/extensions/ directory exists:" if [ -d "./internal/extensions/" ]; then echo "Directory ./internal/extensions/ still exists. Listing remaining test files:" fd -e go -e test.go . ./internal/extensions else echo "Directory ./internal/extensions/ has been removed." fi # List all test files in the new directory echo "Listing test files in ./internal/contracts/tests/:" fd -e go -e test.go . ./internal/contracts/tests/Length of output: 5497
internal/contracts/tests/utils/procedure/execute.go (2)
130-134: LGTM: New input struct is well-defined and consistent.The
DescribeTaxonomiesInputstruct is well-structured and follows the existing patterns in the file. The inclusion of theLatestVersionboolean field appropriately captures the specific requirements for this operation.
129-159: Overall, the changes look good and align with the PR objectives.The new
DescribeTaxonomiesfunction and its input struct have been successfully implemented to address the need for testing composed contracts with start dates. This addition aligns well with the PR objectives and the linked issue (#666).The implementation follows the existing patterns in the file, making it consistent and easy to understand. The only minor suggestion is to consider parameterizing the
Heightfield in theDescribeTaxonomiesfunction for better flexibility and consistency.These changes effectively contribute to enhancing the testing framework for taxonomies within the codebase, as outlined in the PR summary.
internal/contracts/tests/utils/setup/composed.go (5)
22-22: LGTM: TaxonomyDefinitions field type updated appropriatelyThe change from a slice of
TaxonomyItemto theTaxonomystruct for theTaxonomyDefinitionsfield aligns with the updated data model and encapsulates taxonomy information effectively.
158-158: Correct initialization of TaxonomyDefinitionsThe
TaxonomyDefinitionsfield is properly initialized as an emptytypes.Taxonomystruct, preparing it for taxonomy item additions.
Line range hint
176-182: Appending TaxonomyItems correctly to TaxonomyDefinitionsThe code correctly appends
TaxonomyIteminstances toTaxonomyDefinitions.TaxonomyItems, ensuring that each primitive stream is associated with the composed stream along with its weight.
215-219: Updated iteration over TaxonomyItems in setTaxonomyThe loop accurately iterates over
TaxonomyDefinitions.TaxonomyItems, extracting necessary fields for theset_taxonomyprocedure call.
222-226: Ensure 'set_taxonomy' procedure handles 'startDate' parameterThe
set_taxonomyprocedure is now being called with an additionalstartDateargument. Please verify that the procedure definition in the schema has been updated to accept this new parameter and that it handles it correctly. This ensures that the taxonomy start date is properly recorded and utilized in the system.Run the following script to verify that the
set_taxonomyprocedure includes thestartDateparameter:Also applies to: 236-236
internal/contracts/tests/composed_test.go (2)
33-33: Test case for setting taxonomy with start date added correctlyThe new test function
testSetTaxonomyWithStartDateis appropriately added to the test suite.
Line range hint
543-562: EnsurestartDateis correctly handled when nil insetTaxonomyIn the
setTaxonomyfunction, whentaxonomies.StartDateisnil, the variablestartDateremains an empty string. Please confirm that passing an empty string asstartDateto theset_taxonomyprocedure is acceptable and that the procedure can handle it correctly without causing unexpected behavior.internal/contracts/composed_stream_template.kf (1)
782-782: Ensure all calls toset_taxonomyinclude the new$start_dateparameterThe
set_taxonomyprocedure now includes an additional$start_dateparameter. Please verify that all existing calls to this procedure have been updated accordingly to prevent runtime errors due to missing arguments.Run the following script to identify calls to
set_taxonomythat might be missing the$start_dateparameter:
|
@outerlook |
Description
Related Problem
resolves: #666
How Has This Been Tested?
run go test
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Improvements
Chores