fix: adding depency crashes when not string [APE-959]

[wakamex]

What I did

make sure write_test receives a string input.

fixes: #

How I did it

converting only when needed, using an f-string.

How to verify it

see PR in my branch that reverts the fix. lots of tests fail without the fix. functional tests pass with the fix:

Checklist

• Passes all linting checks (pre-commit and CI jobs)
• New test cases have been added and are passing
• Documentation has been updated
• PR title follows Conventional Commit standard (will be automatically included in the changelog)

[wakamex]

not sure this needs a new test, since every old test fails, but here's a minimal version that replicates our project's setup: #103

[fubuloubu]

Hmm, does this handle .contract being None?

[wakamex]

you meant .content right?

I was just looking at how the manifest is loaded in, parse_file looks like it's deprecated: https://github.com/pydantic/pydantic/blob/0e73e00cbf19ad787c685edf01bb1da7c809a13f/pydantic/main.py#L1086

manifest_data = cached_manifest_file.read_text()
manifest_dict = load_dict(manifest_data)
manifest = PackageManifest.validate(manifest_dict)
sources = manifest.sources
first_source = sources[list(sources.keys())[0]]
print(f"{type(first_source)=}")
first_source.content = None
print(f"{str(first_source.content)=}")
print(f"{first_source.content if isinstance(first_source.content, str) else str(first_source.content)=}")
method1 = str(first_source.content)
method2 = first_source.content if isinstance(first_source.content, str) else str(first_source.content)
print(f"{(method1 == method2)=}")

looks like what I wrote is identical to just casting to str, the above gives:

type(first_source)=<class 'ethpm_types.source.Source'>
str(first_source.content)='None'
first_source.content if isinstance(first_source.content, str) else str(first_source.content)='None'
(method1 == method2)=True

[wakamex]

I doubt .content being None would happen, since I think that means the compiler would have given None as output, no? Like on an empty .sol file? I bet it would break before here.

[wakamex]

ah no compiling happens here. tested with a totally empty file (1f14214), and it doesn't pass the if src.content: on line 200:

Content(__root__={})
if src.content:
    print("true")
else:
    print("not")
not

[wakamex]

I found the culprit

pip install eth-ape==0.6.9
pytest -m "not fuzzing"
=============== 43 failed, 11 passed in 22.05s ===============
pip install eth-ape==0.6.8
pytest -m "not fuzzing"
=============== 54 passed in 62.34s (0:01:02) ===============

[wakamex]

narrowed it down to ApeWorX/ape#1337

202640bf164db9a3090011b9d8638cc4ac4afa4f
src.content=Content(__root__={1: '// SPDX-License-Identifier: MIT', 2: '// OpenZeppelin Contracts (last updated v4.8.0) (access/AccessControl.sol)', 3: 

7a9d47c13f62351cec47d45bd04cf0dc5d3c03c6
src.content='// SPDX-License-Identifier: MIT\n// OpenZeppelin Contracts (last updated v4.8.0) (access/AccessControl.sol)\n\npragma solidity ^0.8.0;\n\nimport

[wakamex]

I guess adding tracebacks requires saving manifests with metadata (not just content) which that PR does.

But for manifests you didn't build, they contain just the source code. So src.content doesn't exist, and you gotta just write the whole thing as a string. You can see that in this change:

https://github.com/antazoey/ape/blob/d0e04a45480b43fc892316395a0cc4e469f67eea/src/ape/api/projects.py#L360

It's confusing, because the code already returned the content attribute previously:

https://github.com/antazoey/ape/blob/d0e04a45480b43fc892316395a0cc4e469f67eea/src/ape/api/projects.py#LL351C20-L351C20

I guess the content attribute now has a content attribute?

[antazoey]

there is already a pr for this here #102
can always do str() on content