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

Basic Project Setup + Migrating Code from PySDK PR #298 #1

Merged
merged 87 commits into from Mar 30, 2022
Merged

Conversation

tzaffi
Copy link
Contributor

@tzaffi tzaffi commented Mar 25, 2022

TEAL Blackbox in Graviton

This PR takes over from the now defunct PySDK PR #298

TLDR; Dry Run a Sequence of Inputs on Apps/LogicSigs, View Stats, and make Assertions on Behavior

Please refer to the TEAL Blackbox HowTo / README for full details

A Place to Test Our Python Repo Best Practices

Since this is a brand new repo this is a good place to set some new conventions that our other Python repos should eventually follow also. This PR does not claim to have achieved such a state (especially in light of issues #4 and #7).

Some conventions are implicitly advanced in this PR:

  • no requirements.txt:
    • instead, add an extras_require (e.g. extras_require={"test": "pytest==7.1.1"},) in setup.py
    • install the test-dependencies with pip install -e.[test]
  • pin internal algorand dependencies using version tags
  • (For projects that use github actions) Use act to simulate github actions locally
  • Use Makefile to summarize basic setup/testing commands, and incorporate these commands in CI configs
  • tests should contain all unit and integration tests (as opposed to pairing *_test.py files next to the file they are testing) No longer considered a "best practice"
  • test files should not be installed by default (previous bullet point makes this goal easier)
  • linter configurations should go into setup.cfg whenever possible
  • lint with black --check and flake8

Testing

make integration-test

Simulating Github Actions Locally (on a Mac)

make mac-gh-simulate

TODO

CI / Best Practices

Features / Improvements

Migrate Sandbox Code from PySDK and Run All Tests

Testing is cribbing off of PyTeal's "Run blackbox tests in CI" PR

@tzaffi tzaffi changed the title basic python setup and github attempt with blackbox Basic Project Setup Mar 25, 2022
@tzaffi tzaffi marked this pull request as draft March 25, 2022 14:24
tests/clients.py Outdated Show resolved Hide resolved
setup.cfg Outdated Show resolved Hide resolved
setup.cfg 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 Thanks for tying together a number of threads to bring graviton to life. ☕

For review completeness: I'm on-board to try the proposed versioning scheme though I have lingering concerns. Since we can more freely make changes here, let's try it out and assess over time.

Rationale:

  • It's possible unicode characters interact poorly with other systems (e.g. build systems) and/or are difficult to parse when not presented as emojis (e.g. git tag --list vs git --no-pager tag --list).
  • The convention does not support signaling when a breaking change is made. The lack of signaling makes it harder to know when it's safe to upgrade.

@tzaffi
Copy link
Contributor Author

tzaffi commented Mar 30, 2022

@tzaffi Thanks for tying together a number of threads to bring graviton to life. ☕

For review completeness: I'm on-board to try the proposed versioning scheme though I have lingering concerns. Since we can more freely make changes here, let's try it out and assess over time.

Rationale:

  • It's possible unicode characters interact poorly with other systems (e.g. build systems) and/or are difficult to parse when not presented as emojis (e.g. git tag --list vs git --no-pager tag --list).
  • The convention does not support signaling when a breaking change is made. The lack of signaling makes it harder to know when it's safe to upgrade.

We should keep the new versioning scheme at least through April 1, 2022

Copy link
Member

@jasonpaulos jasonpaulos left a comment

Choose a reason for hiding this comment

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

Comments that didn't make sense on a particular line:

  • I think the animal emoji versioning scheme only make sense if it's mapped directly to a semver version (e.g. 🐈 = 0.1.0). I think it's good marketing, but it should not replace semver and traditional versioning.
  • I'm in favor of having *_test.py unit tests that live in the same directory as implementation code. They are very convenient and act as a constant reminder to write unit tests for your code.

I only shared the above two comments because the context of this repo seems to be to establish best practices for future repos. If that were not the case, my opinions on these issue would not be very strong.

Makefile Outdated Show resolved Hide resolved
Makefile Outdated
pip:
pip install -e .

pip-test: pip
Copy link
Member

Choose a reason for hiding this comment

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

I can accept this, though I'd suggestion development or similar instead of just test. The dev process involves more than just tests, there are also linters, formatters, etc. that help us write code.

setup.cfg Outdated Show resolved Hide resolved
@tzaffi tzaffi merged commit 5549e62 into main Mar 30, 2022
@tzaffi tzaffi deleted the github-actions branch March 30, 2022 22:06
tzaffi pushed a commit that referenced this pull request Oct 31, 2022
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

4 participants