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

fix: make .extract_manifest() also compile if needed #2248

Merged
merged 7 commits into from
Aug 27, 2024
Merged
Show file tree
Hide file tree
Changes from 4 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
14 changes: 10 additions & 4 deletions src/ape/managers/project.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

from ape.api.projects import ApeProject, DependencyAPI, ProjectAPI
from ape.contracts import ContractContainer, ContractInstance
from ape.exceptions import APINotImplementedError, ChainError, ProjectError
from ape.exceptions import APINotImplementedError, ChainError, CompilerError, ProjectError
from ape.logging import logger
from ape.managers.base import BaseManager
from ape.managers.config import ApeConfig
Expand Down Expand Up @@ -1891,6 +1891,12 @@ def reconfigure(self, **overrides):
self.account_manager.test_accounts.reset()

def extract_manifest(self) -> PackageManifest:
# Ensure it compiled (at least attempt).
try:
self.load_contracts()
except CompilerError as err:
logger.debug(err)
Copy link
Member

Choose a reason for hiding this comment

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

Should this be higher? Maybe warning

Copy link
Member Author

Choose a reason for hiding this comment

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

perhaps! I am just worried about dependencies trying to extract manifests from that aren't meant to be compiled.

Copy link
Member Author

Choose a reason for hiding this comment

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

i suppose they could just .manifest in that case.

Copy link
Member Author

Choose a reason for hiding this comment

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

I changed it to warning but added a comment. let me know if that makes sense, along with my concerns.

Copy link
Member

Choose a reason for hiding this comment

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

perhaps! I am just worried about dependencies trying to extract manifests from that aren't meant to be compiled.

Wonder if it makes more sense as like .enrich_manifest or .fully_compiled_manifest, or something


return self.manifest

def clean(self):
Expand Down Expand Up @@ -2460,9 +2466,9 @@ def extract_manifest(self) -> PackageManifest:

sources = dict(self.sources)
contract_types = {
n: ct
for n, ct in (self.manifest.contract_types or {}).items()
if ct.source_id in sources
n: c.contract_type
for n, c in self.load_contracts().items()
if c.contract_type.source_id in sources
}

# Add any remaining data to the manifest here.
Expand Down
41 changes: 41 additions & 0 deletions tests/functional/test_project.py
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,18 @@ def test_extract_manifest_excludes_cache(tmp_project):
assert ".cache/subdir/CacheFile.json" not in (manifest.sources or {})


def test_extract_manifest_compiles(tmp_project):
tmp_project.manifest.contract_types = {} # Not compiled.
actual = tmp_project.extract_manifest()
assert actual.contract_types # Fails if empty


def test_extract_manifest_from_manifest_project(project_from_manifest):
project_from_manifest.manifest.contract_types = {} # Not compiled.
manifest = project_from_manifest.extract_manifest()
assert "FooContractFromManifest" in manifest.contract_types


def test_exclusions(tmp_project):
exclusions = ["Other.json", "*Excl*"]
exclude_config = {"compile": {"exclude": exclusions}}
Expand Down Expand Up @@ -417,6 +429,35 @@ def test_load_contracts_output_abi(tmp_project):
assert isinstance(data[0], dict)


def test_load_contracts_use_cache(mocker, tmp_project):
"""
Showing the 'use_cache=bool' kwarg works.
"""
compile_spy = mocker.spy(tmp_project.contracts, "_compile")

tmp_project.manifest.contract_types = {} # Force initial compile.
contracts = tmp_project.load_contracts(use_cache=True)
assert "Other" in contracts # Other.json contract.
assert "Project" in contracts # Project.json contract.
assert compile_spy.call_args_list[-1][-1]["use_cache"] is True

# Show they get added to the manifest.
assert "Other" in tmp_project.manifest.contract_types
assert "Project" in tmp_project.manifest.contract_types

# Showe we can use the cache again (no compiling!)
contracts = tmp_project.load_contracts(use_cache=True)
assert "Other" in contracts # Other.json contract.
assert "Project" in contracts # Project.json contract.
assert compile_spy.call_args_list[-1][-1]["use_cache"] is True

# Show force-recompiles.
contracts = tmp_project.load_contracts(use_cache=False)
assert "Other" in contracts # Other.json contract.
assert "Project" in contracts # Project.json contract.
assert compile_spy.call_args_list[-1][-1]["use_cache"] is False


def test_manifest_path(tmp_project):
assert tmp_project.manifest_path == tmp_project.path / ".build" / "__local__.json"

Expand Down
Loading