Skip to content
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

Enhancement: Trim the indexer images and use the sandbox instead of custom dockers #367

Merged
merged 65 commits into from
Aug 22, 2022

Conversation

tzaffi
Copy link
Contributor

@tzaffi tzaffi commented Aug 3, 2022

Utilize New Sandbox-based testing SDK and Obsolete Steps

  • Refactoring make docker-test using pure-shell script test-harness.sh that uses the sandbox instead of the old docker scripts
    • NOTE: an attempt has been made to make this easy to re-use in other SDK's with slight modification
  • Removing steps that belong to features being removed in refactored SDK testing

Defining Issue

algorand/algorand-sdk-testing#221

TODO

  • following a conversation with @michaeldiamant - .test-env should have the bare essentials needed to control this SDK's test setup and run. It should not overwrite SDK Testing's .env. This is because the SDK Testing repo has the authority of what branches of go-algorand, indexer, etc. are relevant for a particular feature being tested.
  • create an issue in the sdk-testing repo to improve CI by standing up a test-harness with a simple call to up.shas a github-actions based integrations test. CREATED: https://github.com/algorand/go-algorand-internal/issues/2235
  • Has trim-indexer branch of algorand-sdk-testing been merged?
    • Repoint the dependency here to algorand-sdk-testing/master
    • Now we can merge

At a High Level

make docker-test triggers the following:

  • A. make harness which calls ./test-harness.sh (replacing run_integration.sh):
    1. clone the SDK Testing Repo into ./test-harness/* (renamed from /test)
    2. copy the env vars in .test-env to ./test-harness/.env (this is brand new)
    3. copy the cloned features into ./tests/features/* (same as previous)
    4. run the cloned ./test-harness/up.sh which now does the following:
    • a. use .test-env to interpolate a sandbox config template into a temporary file ./test-harness/_config.harness
      • NOTE: just as previously, there is logic to distinguish between TYPE channel v. source
      • NOTE: there is also logic to handle the quirkiness of sed on mac os versus linux
    • b. clone the sandbox repo into ./test-harness/.sandbox/*
    • c. copy .test-env (via its copy /test-harness/.env) into ./test-harness/.sandbox/.env which serves as a Docker-compatible env file
    • d. move the populated config template into ./test-harness/.sandbox/config.harness
    • e. stand up the sandbox with ./sandbox up -v harness
  • B. make docker-pysdk-build: builds this repo's DockerFile into the image py-sdk-testing
  • C. make docker-pysdk-run: executes the py-sdk-testing which runs all cucumber tests

Summary of Changes

  • .test-env is introduced to configure the test build and the cloned sandbox. During the build it

Step Removals

Please refer to the step removal guide in the companion SDK Testing PR.

Additional Steps being modified due to indexer v1 removal

  • [@rekey_v1 and @send tags] - then("the transaction should go through")
    • DO NOT assert "type" in context.acl.transaction_by_id(context.txn.get_txid()) because this relies on indexer v1
  • [@applications.verified tag] - step("I wait for the transaction to be confirmed.")
    • DO NOT assert "type" in context.acl.transaction_by_id(context.app_txid)

Additional Steps being removed due to Unused Steps Analysis

  • @when("we make a SearchForApplications call with {application_id} and {round}")
  • @when("we make a SearchForApplications call with {application_id} and {round}")
  • @then("Every transaction works with asset-id {assetid}")
  • @then('the parsed response should equal "{jsonfile}".')
  • @then('we expect the path used to be "{path}"')
  • @when("I create the payment transaction")
  • @when('I read a transaction "{txn}" from file "{num}"')
  • @when("I write the transaction to file")
  • @then("the transaction should still be the same")
  • @then("I do my part")
  • @given('key registration transaction parameters {fee} {fv} {lv} "{gh}" "{votekey}" "{selkey}" {votefst} {votelst} {votekd} "{gen}" "{note}"')
  • @when("I create the key registration transaction")

Statistics

Build time average case:

  • latest 5.5 minutes (see the next section)
  • former 7.5 minutes

Running log of build times

  1. Py 3.10 @ 3,000 rounds: 5m 37s
  2. Py 3.10 @ 30,000 rounds FIRST TIME: 6m 18s
  3. Py 3.10 @ 30,000 rounds SECOND TIME: 5m 41s
  4. "" "" THIRD TIME: 6m 8s
  5. Py 3.10 @ 30K rounds + w/o indexer v1 steps: 5m 34s
  6. "" "" "": 5m 35s
  7. "" "" "" (but after sandbox bugfix): 5m 49s
  8. "" "" "" 7m 42s :-( ...not sure if that's a blip or a new trend
  9. "" "" "" 7m 37s ... seems to be a new unfortunate trend

Testing

No functional changes are introduced in this PR. Therefore, the currently running tests are sufficient but should be tested locally in addition to CI.

@tzaffi tzaffi marked this pull request as draft August 3, 2022 23:32
@tzaffi tzaffi changed the title WIP: time printout WIP: Trim the indexer images and use the sandbox instead of custom dockedrs Aug 6, 2022
@tzaffi tzaffi changed the title WIP: Trim the indexer images and use the sandbox instead of custom dockedrs WIP: Trim the indexer images and use the sandbox instead of custom dockers Aug 6, 2022
@tzaffi tzaffi changed the title WIP: Trim the indexer images and use the sandbox instead of custom dockers Enhancement: WIP - Trim the indexer images and use the sandbox instead of custom dockers Aug 6, 2022
test-harness.sh Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
.test-env Outdated Show resolved Hide resolved
test-harness.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@michaeldiamant michaeldiamant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tzaffi Excited to bring in these changes - appreciate the effort! ☕

Copy link
Contributor Author

@tzaffi tzaffi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oopsies

Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
behave --tags=$(INTEGRATIONS) tests -f progress2
behave --tags=$(INTEGRATION_TAGS) tests -f progress2 --no-capture

display-all-python-steps:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

generates text used by the Unused Steps Analysis script

@@ -1,11 +1,24 @@
UNITS = "@unit.abijson or @unit.abijson.byname or @unit.algod or @unit.algod.ledger_refactoring or @unit.applications or @unit.atc_method_args or @unit.atomic_transaction_composer or @unit.dryrun or @unit.dryrun.trace.application or @unit.feetest or @unit.indexer or @unit.indexer.ledger_refactoring or @unit.indexer.logs or @unit.offline or @unit.rekey or @unit.transactions.keyreg or @unit.responses or @unit.responses.231 or @unit.tealsign or @unit.transactions or @unit.transactions.payment or @unit.responses.unlimited_assets or @unit.sourcemap"
UNIT_TAGS := "$(subst :, or ,$(shell awk '{print $2}' tests/unit.tags | paste -s -d: -))"
INTEGRATION_TAGS := "$(subst :, or ,$(shell awk '{print $2}' tests/integration.tags | paste -s -d: -))"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cucumber tags now reside in:

  • tests/unit.tags
  • tests/integration.tags

.test-env Outdated Show resolved Hide resolved
Copy link
Contributor

@michaeldiamant michaeldiamant left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approval assumes #367 (comment) is addressed prior to merge.

Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>
@tzaffi tzaffi merged commit ca75305 into develop Aug 22, 2022
@tzaffi tzaffi deleted the trim-indexer branch August 22, 2022 16:15
ahangsu added a commit that referenced this pull request Sep 2, 2022
* add check to desc so we dont output null if undefined (#368)

* Bumped version to v1.16.1

* Enhancement: Trim the indexer images and use the sandbox instead of custom dockers (#367)

Co-authored-by: algochoi <86622919+algochoi@users.noreply.github.com>
Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>

* Bug-fix: Pass verbosity through to testing harness (#373)

* Enhancement: Add State Proof support (#370)

* add stateproof support

* Enhancement: Deprecating use of langspec  (#371)

* Bumped version to v1.17.0

* Update README.md

Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>

* Mergeback: Release v1.17.0b1 (#378)

* Bumped version
* Update README.md

* bumped version to v1.17.0

Co-authored-by: Ben Guidarelli <ben.guidarelli@gmail.com>
Co-authored-by: Barbara Poon <barbara.poon@algorand.com>
Co-authored-by: Zeph Grunschlag <tzaffi@users.noreply.github.com>
Co-authored-by: algochoi <86622919+algochoi@users.noreply.github.com>
Co-authored-by: Michael Diamant <michaeldiamant@users.noreply.github.com>
Co-authored-by: shiqizng <80276844+shiqizng@users.noreply.github.com>
Co-authored-by: Jack Smith <jack.smith@algorand.com>
Co-authored-by: Jack <87339414+algojack@users.noreply.github.com>
Co-authored-by: Lucky Baar <lucky.baar@algorand.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants