Conversation
Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>
michaeldiamant
left a comment
There was a problem hiding this comment.
@tzaffi Thank you for your investigative work + multiple cycles to bring a net improvement in testing experience + build times!
Thanks for approving. I will not merge this PR unitll all the "TODO" checkboxes in the PR description have been checked - indicating that all the dependencies are in place for merging. |
winder
left a comment
There was a problem hiding this comment.
Great! Thanks for digging into this and figuring out how many deprecated/duplicate tests we have.
Do you have any idea how many steps this removes?
I found this plugin for Java, maybe the other languages have something similar: https://blog.jdriven.com/2019/01/cucumber-jvm-plugin/
Currently, there are 48 steps in the "Steps Removal Guide". @algochoi - 26 thru 48 are the new ones which I identified after I approved the Javascript PR. Most helpful was actually the go-sdk which organized the indexer feature steps all in one place. Next up, I'll be doing Java and will try to use the tool you linked (now I wish I had started with Java first). I'll update the "Steps Removal Guide" with any more deletions that I discover. |
|
|
||
| @compile | ||
| Scenario Outline: Compile programs | ||
| When I compile a teal program <program> |
There was a problem hiding this comment.
Making variable templating more compliant with SDK expectations.
michaeldiamant
left a comment
There was a problem hiding this comment.
@tzaffi Thanks for taking in several rounds of feedback - Looking forward to merging!
Summary
After removing the 4 non-live indexer instances and realizing that the live one wasn't operational, it was easier to swap out all the docker containers with the much more stable sandbox.
TODO
Stop testing all v1 endpoints. This will require significant investigation into what the v1 endpoint-dependencies are inside of SDK steps implementations and implicitly their features.instead of removing features and steps -as is done in this PR- re-tag everything so that obsolete steps are marked@obsolete. This requires much more detailed tagging of steps.Something else?@rekey_v1and@sendtags] -then("the transaction should go through")DO NOTassert "type" in context.acl.transaction_by_id(context.txn.get_txid())because this relies on indexer v1@applications.verifiedtag] -step("I wait for the transaction to be confirmed.")DO NOTassert "type" in context.acl.transaction_by_id(context.app_txid)Related PR's
up.sh's logic (see stepA.iv.)Files introduced/modified/dropped:
.envto help configure the sandbox. This replaces.up-envconfig.harnessis a sandbox template used for standing it updocker-compose.ymlanddocker/***are removed as they are subsumed by sandbox's docker build artifactsfeatures/integration/- removed integration test features:evaldelta.feature- this hasn't been running since February 2022indexer.featureandfeatures/resources/v23x*/***fixtures - this was a static test last modified March 2021 and which didn't interact with algodnetwork_config/- removing all configurations for bootstrapping the algod node. As Sandbox is now in charge of bringing up the network, all configurations are "owned" by it. If in the future, a network that doesn't exist in Sandbox is required by SDK testing, it is recommended that this network be introduced to Sandbox via a pull request.scripts/- restructured the test scripts:restart.sh- this is removed as sandbox already has./sandbox upwhich serves the same purpose in a robust fashiondown.sh- delegates to./sandbox downand./sandbox cleanup.sh- serves a similar purpose as before, delegating logic based onchannel/sourceconfiguration and also populating theconfig.harnesstemplate to be utilized in/sandbox up config.harnessutils/- removing all files except forutils/implements_all_tests.sh. The removed files include:create_indexer_integration_goldens.shtest case generator that depended on the non-live indexers being removedutils/create_transactions.shan old test case generating script.*tealand*tokfiles which weren't used in any tests. These have been subsumed by the programs infeatures/resources/programs/utils/unused_steps_analysis.ipynb- Script which uses a gherkin parser to aggregate all feature steps, and then uses regular expressions and fuzzy string logic to help discover unused steps in each SDK. This helped discover an average of 12 obsolete steps in each of the SDK's. If we were to start running nightly jobs in this repo, and with a bit more TLC, this could be automated to alert of unused steps.Steps Removal Guide
The following steps have been orphaned due to the removal of
indexer.featureandevaldelta.feature:when('I use {indexer} to lookup account "{account}" at round {round}')when('I use {indexer} to search for an account with {assetid}, {limit}, {currencygt}, {currencylt}, "{auth_addr:MaybeString}", {application_id}, "{include_all:MaybeBool}" and token "{token:MaybeString}"')when('I use {indexer} to search for an account with {assetid}, {limit}, {currencygt}, {currencylt}, "{auth_addr:MaybeString}", {application_id} and token "{token:MaybeString}"')when('I use {indexer} to search for an account with {assetid}, {limit}, {currencygt}, {currencylt} and token "{token:MaybeString}"')when("I get the next page using {indexer} to search for an account with {assetid}, {limit}, {currencygt} and {currencylt}")when('I use {indexer} to search for applications with {limit}, {application_id}, "{include_all:MaybeBool}" and token "{token:MaybeString}"')when('I use {indexer} to search for applications with {limit}, {application_id}, and token "{token:MaybeString}"')when('I use {indexer} to lookup application with {application_id} and "{include_all:MaybeBool}"')when("I use {indexer} to lookup application with {application_id}")when("I get the next page using {indexer} to lookup asset balances for {assetid} with {currencygt}, {currencylt}, {limit}")when("I use {indexer} to search for all {assetid} asset transactions")when('I use {indexer} to search for all "{accountid}" transactions')when("I use {indexer} to check the services health")when("I use {indexer} to lookup block {number}")when('I use {indexer} to lookup asset balances for {assetid} with {currencygt}, {currencylt}, {limit} and token "{token}"')when("I use {indexer} to lookup asset {assetid}")when("I get the next page using {indexer} to search for transactions with {limit} and {maxround}")when('I use {indexer} to search for transactions with {limit}, "{noteprefix:MaybeString}", "{txtype:MaybeString}", "{sigtype:MaybeString}", "{txid:MaybeString}", {block}, {minround}, {maxround}, {assetid}, "{beforetime:MaybeString}", "{aftertime:MaybeString}", {currencygt}, {currencylt}, "{address:MaybeString}", "{addressrole:MaybeString}", "{excludecloseto:MaybeString}" and token "{token:MaybeString}"')when('I use {indexer} to search for transactions with {limit}, "{noteprefix:MaybeString}", "{txtype:MaybeString}", "{sigtype:MaybeString}", "{txid:MaybeString}", {block}, {minround}, {maxround}, {assetid}, "{beforetime:MaybeString}", "{aftertime:MaybeString}", {currencygt}, {currencylt}, "{address:MaybeString}", "{addressrole:MaybeString}", "{excludecloseto:MaybeString}", {application_id} and token "{token:MaybeString}"')when('I use {indexer} to search for assets with {limit}, {assetidin}, "{creator:MaybeString}", "{name:MaybeString}", "{unit:MaybeString}", and token "{token:MaybeString}"')when('indexer client {index} at "{address}" port {port} with token "{token}"')then("I get transactions by address only")- needs indexer v1 [@algodtag]then("I can get the transaction by ID")- needs indexer v1 [@algodtag]when("I get recent transactions, limited by {cnt} transactions")- [@algodtag]then("I get transactions by address and date")- [@algodtag]Additional
indexer.featuresteps which were previously overlooked, but need removal.These are uniquely discoverable (unless noted otherwise) by the following search strings:
26.
I receive status code27.
transactions, has the previous block hash28.
has a frozen status of29.
with a total amount of30.
μalgos31.
The asset found has:32.
with the asset, the first33.
, the first has34.
The first account is online and has35.
transactions in the response, the first is36.
Every transaction has tx-type37.
Every transaction has sig-type38.
Every transaction has round >=39.
Every transaction has round <=40.
Every transaction has round41.
Every transaction works with asset-id42.
Every transaction is older than43.
Every transaction is newer than44.
Every transaction moves between45.
assets in the response, the first is46.
the parsed response should equal- CAREFUL!!! there are unit as well as integration test versions. Make sure to only remove the indexer integration test version47.
the unconfirmed pending transaction by ID(actually anevaldeltafeature)48.
the confirmed pending transaction by IDSteps being modified (for example in the Python SDK)
@rekey_v1and@sendtags] -then("the transaction should go through")assert "type" in context.acl.transaction_by_id(context.txn.get_txid())because this relies on indexer v1@applications.verifiedtag] -step("I wait for the transaction to be confirmed.")assert "type" in context.acl.transaction_by_id(context.app_txid)