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

docs: add doctesting to userguides #2024

Closed
wants to merge 11 commits into from
Closed

Conversation

dtdang
Copy link
Contributor

@dtdang dtdang commented Apr 25, 2024

What I did

Add doctesting to the documentation userguides.

fixes: #657

How I did it

How to verify it

Checklist

  • All changes are completed
  • New test cases have been added
  • Documentation has been updated

docs/conf.py Outdated
)

doctest_global_setup = '''
from ape import accounts
Copy link
Member

Choose a reason for hiding this comment

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

we likely want to import all the ape top-level stuff here.

Copy link
Member

Choose a reason for hiding this comment

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

from ape import *?

Copy link
Member

@antazoey antazoey Apr 26, 2024

Choose a reason for hiding this comment

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

that works! a good use-case for the *-style import

docs/conf.py Outdated
doctest_global_setup = '''
from ape import accounts

my_account_alias = accounts.test_accounts[0]
Copy link
Member

Choose a reason for hiding this comment

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

question: are we able to have individual test setups? something like this doesnt need to be global
issue: i commented elsewhere, but this is supposed to be an alias, not an account, and i think itll likely always fail, ideally we ignore it because it is like a "task failed successfully" kinda moment. but otherwise, you could try my_account_alias = accounts.test_accounts[0].alias

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes we can do individual test setups. I was trying this out to test if it would be nicer for this to have in a global setup but I'll switch it to individual since it's currently not being used for more than one test.

```

**NOTE**: You can also deploy contracts from the container itself:

```python
from ape import accounts, project

account = accounts.load("my_account_alias")
account = accounts.load(f"{my_account_alias}")
Copy link
Member

Choose a reason for hiding this comment

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

this will likely always fail in a doc-test i think that is ok. is there a way to ignore all ApeExceptions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes currently this is failing but if my_account_alias = accounts.test_accounts[0].alias works in the testsetup then I can try and use that instead. I'll look into ignoring ApeExceptions since that would be also useful in a few other userguides.

@dtdang dtdang changed the title Docs: add doctesting to userguides docs: add doctesting to userguides Apr 25, 2024
@fubuloubu
Copy link
Member

side note: makes it a little harder to review since github does not render the markdowns as well, would be helpful to see a generated html file

@antazoey
Copy link
Member

side note: makes it a little harder to review since github does not render the markdowns as well, would be helpful to see a generated html file

you just have to build the docs locally and look at them! we could have docs get build and deployed on PRs, maybe by special command, or maybe if it detect changes to any docs file... that is cool but a bit over-the-top imo, just build them python build_docs.py and then pythom -m something something it is in the contrib.

@fubuloubu
Copy link
Member

side note: makes it a little harder to review since github does not render the markdowns as well, would be helpful to see a generated html file

you just have to build the docs locally and look at them! we could have docs get build and deployed on PRs, maybe by special command, or maybe if it detect changes to any docs file... that is cool but a bit over-the-top imo, just build them python build_docs.py and then pythom -m something something it is in the contrib.

punt this down the road, but would be nice to have a temporary render of each PR's docs updates to see it. can just deploy to a bucket

fubuloubu
fubuloubu previously approved these changes May 23, 2024
contract_json = contract_path.read_text()

contract = ContractType.model_validate_json(contract_json)
# project._cached_projects["MyContract"] = contract
Copy link
Member

Choose a reason for hiding this comment

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

is this not needed?

Suggested change
# project._cached_projects["MyContract"] = contract

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is necessary but I'm still working on storing the contract as a cached project to be used in other examples.

Copy link
Member

Choose a reason for hiding this comment

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

i think you an do chain.contracts[address] = contract_type.

contract = Contract("v2.registry.ychad.eth")
>>> from ape import Contract

>>> contract = Contract("v2.registry.ychad.eth")
Copy link
Member

Choose a reason for hiding this comment

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

this will fail if ape-ens and ape-etherscan plugins are no installed, which i dont think they are in CI

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will remove this doctest then.

>>> # Same as doing `networks.ethereum.local.use_provider("test")`.
>>> with networks.parse_network_choice("ethereum:local:test") as provider:
... print(provider.name)
test
Copy link
Member

Choose a reason for hiding this comment

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

what is this final test here? (the word)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The final test here is the output string that the doctest is expecting to compare to the results from the test above.

Copy link

This pull request is considered stale because it has been open 30 days with no activity. Remove stale label, add a comment, or make a new commit, otherwise this PR will be closed in 5 days.

@github-actions github-actions bot added the stale No activity for 30 days label Jul 11, 2024
Copy link

This PR was closed because it has been inactive for 35 days.

@github-actions github-actions bot added the inactive no recent activity, closed label Jul 16, 2024
@github-actions github-actions bot closed this Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
inactive no recent activity, closed stale No activity for 30 days
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Doc Tests
3 participants