Skip to content

refactor environment variables handling and remove unused code #33

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.

Sign up for GitHub

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

Open
0xjunha wants to merge 2 commits into
base: main
Choose a base branch
from
Open

refactor environment variables handling and remove unused code #33

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1a98880
refactor environment variables handling and remove unused code
0xjunha Sep 27, 2024
d777aa2
remove `APTOS_INDEXER_URL` config from the setUpClass method of `inte…
0xjunha Sep 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ make test
* Download and install the [Aptos CLI](https://aptos.dev/tools/aptos-cli/use-cli/running-a-local-network).
* Set the environment variable `APTOS_CLI_PATH` to the full path of the CLI.
* Retrieve the [Aptos Core Github Repo](https://github.com/aptos-labs/aptos-core) (git clone https://github.com/aptos-labs/aptos-core)
* Set the environment variable `APTOS_CORE_REPO` to the full path of the Repository.
* Set the environment variable `APTOS_CORE_PATH` to the full path of the Repository.
* `make integration_test`

You can do this a bit more manually by:
Expand All @@ -41,7 +41,7 @@ aptos node run-local-testnet --force-restart --assume-yes --with-indexer-api
Next, tell the end-to-end tests to talk to this locally running testnet:

```bash
export APTOS_CORE_REPO="/path/to/repo"
export APTOS_CORE_PATH="/path/to/repo"
export APTOS_FAUCET_URL="http://127.0.0.1:8081"
export APTOS_INDEXER_URL="http://127.0.0.1:8090/v1/graphql"
export APTOS_NODE_URL="http://127.0.0.1:8080/v1"
Expand Down
5 changes: 1 addition & 4 deletions examples/common.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,6 @@
"APTOS_FAUCET_URL",
"https://faucet.devnet.aptoslabs.com",
)
INDEXER_URL = os.getenv(
"APTOS_INDEXER_URL",
"https://api.devnet.aptoslabs.com/v1/graphql",
)
INDEXER_URL = os.getenv("APTOS_INDEXER_URL")
Copy link
Contributor

@davidiw davidiw Oct 1, 2024

Choose a reason for hiding this comment

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

why remove this?

Copy link
Contributor Author

@0xjunha 0xjunha Oct 2, 2024

Choose a reason for hiding this comment

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

This first looks into APTOS_INDEXER_URL, but in the case user is running localnet without the --with-indexer-api (and without APTOS_INDEXER_URL) this default value will give us the devnet indexer url, when we're running local tests without an indexer.

Test cases using indexer like test_transfer_coin fails if we keep this default value.

NODE_URL = os.getenv("APTOS_NODE_URL", "https://api.devnet.aptoslabs.com/v1")
# <:!:section_1
26 changes: 14 additions & 12 deletions examples/integration_test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,24 +13,26 @@
from aptos_sdk.account_address import AccountAddress
from aptos_sdk.aptos_cli_wrapper import AptosCLIWrapper, AptosInstance

from .common import APTOS_CORE_PATH


class Test(unittest.IsolatedAsyncioTestCase):
_node: Optional[AptosInstance] = None
_aptos_core_path: str
Copy link
Contributor

@davidiw davidiw Oct 1, 2024

Choose a reason for hiding this comment

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

why this refactor?

Copy link
Contributor Author

@0xjunha 0xjunha Oct 2, 2024

Choose a reason for hiding this comment

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

If we import common.py here,

os.environ["APTOS_FAUCET_URL"] = "http://127.0.0.1:8081"
os.environ["APTOS_NODE_URL"] = "http://127.0.0.1:8080/v1"

of setUpClass method in integration_test.py will not be applied in each test case.

So make integration_test is basically running tests on devnet right now.

Copy link
Contributor

@davidiw davidiw Oct 7, 2024

Choose a reason for hiding this comment

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

Hmm... because it is setUpClass, let's move these to setUp then. Good call!


@classmethod
def setUpClass(self):
def setUpClass(cls):
cls._aptos_core_path = os.getenv("APTOS_CORE_PATH")
if not cls._aptos_core_path:
raise Exception("Environment variable `APTOS_CORE_PATH` is not set")

if os.getenv("APTOS_TEST_USE_EXISTING_NETWORK"):
return

self._node = AptosCLIWrapper.start_node()
operational = asyncio.run(self._node.wait_until_operational())
cls._node = AptosCLIWrapper.start_node()
operational = asyncio.run(cls._node.wait_until_operational())
if not operational:
raise Exception("".join(self._node.errors()))
raise Exception("".join(cls._node.errors()))

os.environ["APTOS_FAUCET_URL"] = "http://127.0.0.1:8081"
os.environ["APTOS_INDEXER_CLIENT"] = "none"
Copy link
Contributor

@davidiw davidiw Oct 21, 2024

Choose a reason for hiding this comment

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

so what you're saying is that this doesn't actually set the environment variable and it is overwritten each time in common.py?

That doesn't make a lot of sense according to the docs.

The way it should work is that we set this once during the class call to "none", each common.py should see that it is set to "none" and leave it alone, during loading of the indexer client we should see it is "none" and not touch it.

os.environ["APTOS_NODE_URL"] = "http://127.0.0.1:8080/v1"

async def test_aptos_token(self):
Expand All @@ -47,7 +49,7 @@ async def test_hello_blockchain(self):
from . import hello_blockchain

hello_blockchain_dir = os.path.join(
APTOS_CORE_PATH, "aptos-move", "move-examples", "hello_blockchain"
self._aptos_core_path, "aptos-move", "move-examples", "hello_blockchain"
)
AptosCLIWrapper.test_package(
hello_blockchain_dir, {"hello_blockchain": AccountAddress.from_str("0xa")}
Expand All @@ -62,7 +64,7 @@ async def test_large_package_publisher(self):
from . import large_package_publisher

large_packages_dir = os.path.join(
APTOS_CORE_PATH, "aptos-move", "move-examples", "large_packages"
self._aptos_core_path, "aptos-move", "move-examples", "large_packages"
)
module_addr = await large_package_publisher.publish_large_packages(
large_packages_dir
Expand Down Expand Up @@ -121,19 +123,19 @@ async def test_your_coin(self):
from . import your_coin

moon_coin_path = os.path.join(
APTOS_CORE_PATH, "aptos-move", "move-examples", "moon_coin"
self._aptos_core_path, "aptos-move", "move-examples", "moon_coin"
)
AptosCLIWrapper.test_package(
moon_coin_path, {"MoonCoin": AccountAddress.from_str("0xa")}
)
await your_coin.main(moon_coin_path)

@classmethod
def tearDownClass(self):
def tearDownClass(cls):
if os.getenv("APTOS_TEST_USE_EXISTING_NETWORK"):
return

self._node.stop()
cls._node.stop()


if __name__ == "__main__":
Expand Down
8 changes: 2 additions & 6 deletions examples/multikey.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
from aptos_sdk.account import Account
from aptos_sdk.account_address import AccountAddress
from aptos_sdk.asymmetric_crypto_wrapper import MultiSignature, Signature
from aptos_sdk.async_client import FaucetClient, IndexerClient, RestClient
from aptos_sdk.async_client import FaucetClient, RestClient
from aptos_sdk.authenticator import AccountAuthenticator, MultiKeyAuthenticator
from aptos_sdk.bcs import Serializer
from aptos_sdk.transactions import (
Expand All @@ -17,17 +17,13 @@
TransactionPayload,
)

from .common import FAUCET_URL, INDEXER_URL, NODE_URL
from .common import FAUCET_URL, NODE_URL


async def main():
# :!:>section_1
rest_client = RestClient(NODE_URL)
faucet_client = FaucetClient(FAUCET_URL, rest_client) # <:!:section_1
if INDEXER_URL and INDEXER_URL != "none":
IndexerClient(INDEXER_URL)
else:
pass

# :!:>section_2
key1 = secp256k1_ecdsa.PrivateKey.random()
Expand Down