Add Sdk tests #554
Add Sdk tests #554
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
yuunlimm
force-pushed
the
10-16-add_sdk_tests
branch
from
f80db7e to
0a38648
Compare
| .unwrap_or_else(|| DEFAULT_OUTPUT_FOLDER.to_string() + "imported_testnet_txns"); | ||
|
|
||
| // Step 1: set up an input transaction that will be used | ||
| let transaction_batches = ALL_IMPORTED_TESTNET_TXNS |
There was a problem hiding this comment.
Instead of running the events processor test on every imported transaction, would it be better to let the user decide?
I liked the devex we had before where for each cargo test, you defined which transactions you want to use. Running each processor test on all imported transactions would create a bunch of empty files / no-ops and aren't super useful.
There was a problem hiding this comment.
+1 what's the reason to run ALL_IMPORTED_TESTNET_TXNS? Can this just be EVENTS_PROCESSOR_IMPORTED_TESTNET_TXNS?
There was a problem hiding this comment.
they are the next milestones, that test case is there to make sure that we can process them without any issues.
more test cases with purpose and context could be added as a part of the next milestone: 90% coverage
There was a problem hiding this comment.
Testing each processor with ALL_IMPORTED_TESTNET_TXNS can get us to a high % test coverage goal for sure, but you lose the context associated with the transaction and the test, and I'm not sure how debuggable these are.
I think we should be importing transactions and testing intentionally as part of first milestone. The bulk of the work is just identifying which transactions to use, right? Since we have this framework now it should be really easy to add tests.
There was a problem hiding this comment.
that test case is there to make sure that we can process them without any issues.
Can we instead then do [EVENTS_PROCESSOR_IMPORTED_TESTNET_TXNS + IRRELEVANT_TXN] with a comment so that this intent is clear?
There was a problem hiding this comment.
you are absolutely right, most of the effort now lies in identifying which transactions to use for testing specific scenarios. and using ALL_IMPORTED_TESTNET_TXNS can certainly help us reach the initial test coverage milestone faster. It’s a valuable proof of concept that shows our system is correctly indexing large sets of real-world data, even if it lacks the intentionality and debuggability of more focused tests.
here’s how I think we can position this:
- ALL_IMPORTED_TESTNET_TXNS as a Milestone:
- It serves as a strong baseline, proving that we can handle and index transactions correctly. Even though it doesn't focus on specific scenarios, it's a great starting point when we didn’t have any coverage.
- By running all imported transactions, we can show that our system processes real-world data as expected. This is useful as an overall system health check.
- Next Milestones with More Intentional Tests:
- Once we have the initial proof of correct indexing, the next milestone can focus on intentional testing, which will bring in more context and debugability.
- We can target specific edge cases, error conditions, and SEVs that have been critical in the past, aligning this with the goal of reaching 90% coverage.
This gives us a strong base with test coverage and indexing proof, while also allowing room to refine and add higher-quality tests in the next phases. Would this align with your thoughts?
There was a problem hiding this comment.
@yuunlimm I hear ya here . Let's leave a TODO for the next milestone so we don't forget to update this later?
yuunlimm
force-pushed
the
10-16-update_existing_processor_tests_with_the_latest_dependency_change
branch
from
62c78b1 to
b95f3da
Compare
yuunlimm
force-pushed
the
10-16-add_sdk_tests
branch
2 times, most recently
from
006e9aa to
7fa5c46
Compare
There was a problem hiding this comment.
Stamping to unblock, let's add the todo that @dermanyang suggested!
yuunlimm
force-pushed
the
10-16-add_sdk_tests
branch
from
7fa5c46 to
7648c32
Compare
* Add Sdk tests (#554) * support multi txns testing * rebase and update dependency
* Add Sdk tests (#554) * update manipulated version * split tests * update tests
#545