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

New Feature: Implemented disassemble method #622

Closed
wants to merge 1 commit into from
Closed

New Feature: Implemented disassemble method #622

wants to merge 1 commit into from

Conversation

kael-shipman
Copy link

This implementation closes #621

@kael-shipman
Copy link
Author

I couldn't figure out how to write tests for this, so I would appreciate a push in the right direction. I searched the tests directory for compile to try to just copy those tests, and I found some in the cucumber directory, but I couldn't make sense of them. Any help is appreciated.

@barnjamin barnjamin changed the title Implemented disassemble method New Feature: Implemented disassemble method Aug 10, 2022
@barnjamin
Copy link
Contributor

barnjamin commented Aug 10, 2022

nice! thanks for submitting this.

Tests are written for all the sdks in this repo https://github.com/algorand/algorand-sdk-testing/

Once written you can tweak the git clone line in https://github.com/algorand/js-algorand-sdk/blob/develop/tests/cucumber/docker/run_docker.sh to point to your copy and run it to pull the tests down

Then enable the tagged tests in the Makefile by adding or @your.test or commenting the rest of them out and adding yours alone for faster iteration.

@michaeldiamant anything to add?

@michaeldiamant
Copy link
Contributor

@kael-shipman Thanks for offering a contribution!

Let us know if the above note + this note help provide a stating point.

@michaeldiamant anything to add?

Linking to a recent test addition example + corresponding implementation in js-algorand-sdk:

The example illustrates a couple of facets:

  • Adding a custom tag (@compile.sourcemap) to support adding functionality in each SDK incrementally.
  • Unit + integration style test examples to illustrate typical test setup and scope.

@kael-shipman
Copy link
Author

Ah, thanks! I saw this comment but marked it "read" and forgot to come back to it. I'll see if I have time today to add some tests. Thanks for the push!

@tzaffi
Copy link
Contributor

tzaffi commented Sep 7, 2022

nice! thanks for submitting this.

Tests are written for all the sdks in this repo https://github.com/algorand/algorand-sdk-testing/

Once written you can tweak the git clone line in https://github.com/algorand/js-algorand-sdk/blob/develop/tests/cucumber/docker/run_docker.sh to point to your copy and run it to pull the tests down

Then enable the tagged tests in the Makefile by adding or @your.test or commenting the rest of them out and adding yours alone for faster iteration.

@michaeldiamant anything to add?

Update: we have recently refactored the way tests work. The general approach described by @barnjamin remains the same. But there is no longer a run_docker.sh script where you would tweak the git clone branch of sdk-testing. Instead, this is managed in .test-env. You would also need merge the latest from develop into this pull request.

@kael-shipman
Copy link
Author

Sorry, I've moved on to other things and don't know if I'll get a chance to figure out how testing works for this. I took a look at it last week and it wasn't immediately obvious to me how to write test cases, although the examples were helpful. Given that, you can feel free to close this PR if no one is available to fill in the tests.

@michaeldiamant
Copy link
Contributor

@kael-shipman Thanks for the update. I appreciate you took a look at the testing methodology and totally understand that it's a greater time investment than you have available now. Closing the PR for now.

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.

AlgodV2: Add disassemble/decompile function to correspond with new disassemble endpoint
4 participants