search

ApeWorX / ape

The smart contract development tool for Pythonistas, Data Scientists, and Security Professionals
https://apeworx.io
Apache License 2.0
845 stars 124 forks source link

(ape-safe) tests - Safe contracts are not cached #2133

Closed banteg closed 6 days ago

banteg commented 2 weeks ago

What went wrong?

every time i run ape test i see

Downloading safe-global/safe-smart-account@v1.3.0: : 2.93MiB [00:02, 1.35MiB/s]

this line makes me think github deps are not cached.

you can try this yourself in apeworx/ape-safe repo.

How can it be fixed?

cache the deps

linear[bot] commented 2 weeks ago

APE-1763 github dependencies are not cached

antazoey commented 2 weeks ago

dependencies are cached, this is probably related to the tests in ape-safe. can we move this issue there?

antazoey commented 2 weeks ago

https://github.com/ApeWorX/ape-safe/blob/main/tests/conftest.py#L22-L25

fubuloubu commented 2 weeks ago

https://github.com/ApeWorX/ape-safe/blob/main/tests/conftest.py#L22-L25

good catch!

we can probably give it like a standard name under cache? or I guess there's really not a problem in just using the default. I think I did this when I was having problems getting safe-contracts to install properly

antazoey commented 2 weeks ago

we can probably give it like a standard name under cache? or I guess there's really not a problem in just using the default. I think I did this when I was having problems getting safe-contracts to install properly

This isn't the perfect solution but this is how I handled it in ape-solidity: https://github.com/ApeWorX/ape-solidity/blob/main/tests/conftest.py#L79-L95

fubuloubu commented 2 weeks ago

I actually think it would be fine to cache these dependencies here, both for local testing and CI

banteg commented 2 weeks ago

this is how I handled it in ape-solidity

this looks like something that can be handled in ape itself. maybe we can have some utility function for testing that can either fork the data folder, making all installed packages available, or make a clean one.

but i think this also doesn't address an issue when you don't have the package installed and it gets installed when running tests. this needs to be cached too!

antazoey commented 2 weeks ago

but i think this also doesn't address an issue when you don't have the package installed and it gets installed when running tests. this needs to be cached too!

Dependencies during tests do get cached, just they get cached to a temporary data folder. (just highlighting this because Ape's caching system does not care when something is cached - tests, scripts, cli whatever - the only issue here is the tests are in ape-safe and changing where the cache lives and it is not a bug in Ape)

antazoey commented 2 weeks ago

this looks like something that can be handled in ape itself.

maybe this'll help: https://github.com/ApeWorX/ape/pull/2139