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

test: showing dependencies works in project.extract_manifest() #2236

Closed
wants to merge 1 commit into from
Closed
Changes from all commits
Commits
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
38 changes: 34 additions & 4 deletions tests/functional/test_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,11 +261,41 @@ def test_extract_manifest(tmp_project, mock_sepolia, vyper_contract_instance):
assert PackageName("manifest-dependency") in (manifest.dependencies or {})
bip122_chain_id = to_hex(tmp_project.provider.get_block(0).hash)
expected_uri = f"blockchain://{bip122_chain_id[2:]}"
for key in manifest.deployments or {}:
if key.startswith(expected_uri):
return

assert False, "Failed to find expected deployment URI"
# Prove that deployments are available on an extract-manifest.
found = False
for key, val in (manifest.deployments or {}).items():
if key.startswith(expected_uri):
assert "VyperContract.json" in val
actual_deployment = val["VyperContract.json"]
assert actual_deployment.contract_type == "contracts/VyperContract.vy:VyperContract"
assert actual_deployment.address == vyper_contract_instance.address
found = True
break

assert found, "Failed to find expected deployment URI"

# Prove that dependencies works.
Copy link
Member

Choose a reason for hiding this comment

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

These are all local package dependencies?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, but chatting with The God of Ninjas, I have learned to try stuff with snekmate, so going to experiment there.

Copy link
Member

Choose a reason for hiding this comment

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

It may be the case we just don't have a good way to share non-local dependencies using EthPM, and so we can just ask for additional dependencies when compiling in a more ape-friendly config format to use in his service

Copy link
Member Author

Choose a reason for hiding this comment

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

As long as it's in your config, it should work, it just turns to a URL format.

Copy link
Member

Choose a reason for hiding this comment

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

But it has to be the URL of an EthPM Manifest (which is not very common)

Copy link
Member Author

Choose a reason for hiding this comment

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

Why just be a link to a directory containing the files? That is how it works now, and ape puts them in a manifest

Copy link
Member

Choose a reason for hiding this comment

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

This isn't really how it's meant to work, which is why EthPM needs to be fundamentally re-thought anyways. Dependencies are best as like links to a zip file, ipfs directory, or other filesystem-like resource that is globally reference-able and contains sources to use (and potentially compiler settings, compiled artifacts, and other reference metadata that may be useful too) so a downstream project can describe its build manifest in full

For the hosted-compiler we can just use a list of ape-compatible dependency objects which we can push into an ape-config to use to compile with

Copy link
Member Author

Choose a reason for hiding this comment

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

FWIW, Ape is getting better at deducing settings from a project structure, such as Foundry projects.
So if you have a URL, you can fetch it and ape can do Project(path).extract_manifest()

Copy link
Member Author

Choose a reason for hiding this comment

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

snekmate includes absolutely nothing besides contracts in their published PyPI package, which is frustrating. I told them that and they didn't care.

Copy link
Member

Choose a reason for hiding this comment

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

Technically, a Python wheel is a zip file at some URI on the pypi server

# NOTE: tmp_project has several dependencies!
expected_dependencies = (
"default",
"renamed-contracts-folder",
"renamed-complex-contracts-folder",
"containing-sub-dependencies",
"renamed-contracts-folder-specified-in-config",
"manifest-dependency",
"empty-dependency",
)
actual_dependencies = manifest.dependencies or {}
for expected_dependency in expected_dependencies:
pkg_name = PackageName(expected_dependency)
assert pkg_name in actual_dependencies
# Show str works too.
assert expected_dependency in actual_dependencies
actual_dependency = actual_dependencies[pkg_name]
assert isinstance(actual_dependency, Url)
# NOTE: These dependencies are all local-file based.
assert str(actual_dependency).startswith("file://")


def test_extract_manifest_when_sources_missing(tmp_project):
Expand Down
Loading