From cd91d2da48aed555872481e9f7f4f2f274bd5a23 Mon Sep 17 00:00:00 2001 From: Juliya Smith Date: Tue, 27 Aug 2024 13:22:06 -0500 Subject: [PATCH 1/2] fix: if only 1 error just raise that --- src/ape/managers/compilers.py | 9 +++- tests/functional/conftest.py | 86 ++++++++++++++++-------------- tests/functional/test_compilers.py | 85 ++++++++++++++++++++++++++--- 3 files changed, 132 insertions(+), 48 deletions(-) diff --git a/src/ape/managers/compilers.py b/src/ape/managers/compilers.py index 76552680cb..a57e5f7f13 100644 --- a/src/ape/managers/compilers.py +++ b/src/ape/managers/compilers.py @@ -157,11 +157,18 @@ def compile( errors.append(err) continue - if errors: + if len(errors) == 1: + # If only 1 error, just raise that. + raise errors[0] + + elif len(errors) > 1: + # Raise a combined error. formatted_errors = [f"{e}" for e in errors] error_message = "\n\n".join(formatted_errors) raise CompilerError(error_message) + # else: successfully compiled everything! + def compile_source( self, compiler_name: str, diff --git a/tests/functional/conftest.py b/tests/functional/conftest.py index 54d35a170e..05add6558b 100644 --- a/tests/functional/conftest.py +++ b/tests/functional/conftest.py @@ -634,45 +634,53 @@ def method_abi_with_struct_input(): @pytest.fixture -def mock_compiler(mocker): - mock = mocker.MagicMock() - mock.name = "mock" - mock.ext = ".__mock__" - mock.tracked_settings = [] - mock.ast = None - mock.pcmap = None - - def mock_compile(paths, project=None, settings=None): - settings = settings or {} - mock.tracked_settings.append(settings) - result = [] - for path in paths: - if path.suffix == mock.ext: - name = path.stem - code = to_hex(123) - data = { - "contractName": name, - "abi": mock.abi, - "deploymentBytecode": code, - "sourceId": f"{project.contracts_folder.name}/{path.name}", - } - if ast := mock.ast: - data["ast"] = ast - if pcmap := mock.pcmap: - data["pcmap"] = pcmap - - # Check for mocked overrides - overrides = mock.overrides - if isinstance(overrides, dict): - data = {**data, **overrides} - - contract_type = ContractType.model_validate(data) - result.append(contract_type) - - return result - - mock.compile.side_effect = mock_compile - return mock +def mock_compiler(make_mock_compiler): + return make_mock_compiler() + + +@pytest.fixture +def make_mock_compiler(mocker): + def fn(name="mock"): + mock = mocker.MagicMock() + mock.name = "mock" + mock.ext = f".__{name}__" + mock.tracked_settings = [] + mock.ast = None + mock.pcmap = None + + def mock_compile(paths, project=None, settings=None): + settings = settings or {} + mock.tracked_settings.append(settings) + result = [] + for path in paths: + if path.suffix == mock.ext: + name = path.stem + code = to_hex(123) + data = { + "contractName": name, + "abi": mock.abi, + "deploymentBytecode": code, + "sourceId": f"{project.contracts_folder.name}/{path.name}", + } + if ast := mock.ast: + data["ast"] = ast + if pcmap := mock.pcmap: + data["pcmap"] = pcmap + + # Check for mocked overrides + overrides = mock.overrides + if isinstance(overrides, dict): + data = {**data, **overrides} + + contract_type = ContractType.model_validate(data) + result.append(contract_type) + + return result + + mock.compile.side_effect = mock_compile + return mock + + return fn @pytest.fixture diff --git a/tests/functional/test_compilers.py b/tests/functional/test_compilers.py index 92542d5f24..e252fb0018 100644 --- a/tests/functional/test_compilers.py +++ b/tests/functional/test_compilers.py @@ -71,7 +71,18 @@ def test_flatten_contract(compilers, project_with_contract): compilers.flatten_contract(Path("contract.foo")) -def test_contract_type_collision(compilers, project_with_contract, mock_compiler): +@pytest.mark.parametrize("factory", (str, Path)) +def test_compile(compilers, project_with_contract, factory): + """ + Testing both stringified paths and path-object paths. + """ + path = next(iter(project_with_contract.sources.paths)) + actual = compilers.compile((factory(path),)) + contract_name = path.stem + assert contract_name in [x.name for x in actual] + + +def test_compile_contract_type_collision(compilers, project_with_contract, mock_compiler): _ = compilers.registered_compilers # Ensures cached property is set. # Hack in our mock compiler. @@ -98,13 +109,16 @@ def test_contract_type_collision(compilers, project_with_contract, mock_compiler del compilers.__dict__["registered_compilers"][mock_compiler.ext] +def test_compile_empty(compilers): + # Also, we are asserting it does no fail. + assert list(compilers.compile([])) == [] + + def test_compile_with_settings(mock_compiler, compilers, project_with_contract): new_contract = project_with_contract.path / f"AMockContract{mock_compiler.ext}" new_contract.write_text("foobar", encoding="utf8") settings = {"mock": {"foo": "bar"}} - _ = compilers.registered_compilers # Ensures cached property is set. - # Hack in our mock compiler. compilers.__dict__["registered_compilers"][mock_compiler.ext] = mock_compiler @@ -119,11 +133,66 @@ def test_compile_with_settings(mock_compiler, compilers, project_with_contract): assert actual == settings["mock"] -def test_compile_str_path(compilers, project_with_contract): - path = next(iter(project_with_contract.sources.paths)) - actual = compilers.compile((str(path),)) - contract_name = path.stem - assert contract_name in [x.name for x in actual] +def test_compile_errors(mock_compiler, compilers, project_with_contract): + new_contract = project_with_contract.path / f"AMockContract{mock_compiler.ext}" + new_contract.write_text("foobar", encoding="utf8") + + class MyCustomCompilerError(CompilerError): + pass + + mock_compiler.compile.side_effect = MyCustomCompilerError + _ = compilers.registered_compilers # Ensures cached property is set. + # Hack in our mock compiler. + compilers.__dict__["registered_compilers"][mock_compiler.ext] = mock_compiler + + try: + with pytest.raises(MyCustomCompilerError): + list(compilers.compile([new_contract], project=project_with_contract)) + + finally: + if mock_compiler.ext in compilers.__dict__.get("registered_compilers", {}): + del compilers.__dict__["registered_compilers"][mock_compiler.ext] + + +def test_compile_multiple_errors( + mock_compiler, make_mock_compiler, compilers, project_with_contract +): + """ + Simulating getting errors from multiple compilers. + We should get all the errors. + """ + second_mock_compiler = make_mock_compiler("mock2") + new_contract_0 = project_with_contract.path / f"AMockContract{mock_compiler.ext}" + new_contract_0.write_text("foobar", encoding="utf8") + new_contract_1 = project_with_contract.path / f"AMockContract{second_mock_compiler.ext}" + new_contract_1.write_text("foobar2", encoding="utf8") + + expected_0 = "this is expected message 0" + expected_1 = "this is expected message 1" + + class MyCustomCompilerError0(CompilerError): + def __init__(self): + super().__init__(expected_0) + + class MyCustomCompilerError1(CompilerError): + def __init__(self): + super().__init__(expected_1) + + mock_compiler.compile.side_effect = MyCustomCompilerError0 + second_mock_compiler.compile.side_effect = MyCustomCompilerError1 + _ = compilers.registered_compilers # Ensures cached property is set. + # Hack in our mock compilers. + compilers.__dict__["registered_compilers"][mock_compiler.ext] = mock_compiler + compilers.__dict__["registered_compilers"][second_mock_compiler.ext] = second_mock_compiler + + try: + match = rf"{expected_0}\n\n{expected_1}" + with pytest.raises(CompilerError, match=match): + list(compilers.compile([new_contract_0, new_contract_1], project=project_with_contract)) + + finally: + if mock_compiler.ext in compilers.__dict__.get("registered_compilers", {}): + del compilers.__dict__["registered_compilers"][mock_compiler.ext] def test_compile_source(compilers): From e0fa13353d6900c4a800760df1a35f2845d4c79c Mon Sep 17 00:00:00 2001 From: Juliya Smith Date: Tue, 27 Aug 2024 14:00:21 -0500 Subject: [PATCH 2/2] test: missed cleanup --- tests/functional/test_compilers.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/functional/test_compilers.py b/tests/functional/test_compilers.py index e252fb0018..149d2b119e 100644 --- a/tests/functional/test_compilers.py +++ b/tests/functional/test_compilers.py @@ -191,8 +191,9 @@ def __init__(self): list(compilers.compile([new_contract_0, new_contract_1], project=project_with_contract)) finally: - if mock_compiler.ext in compilers.__dict__.get("registered_compilers", {}): - del compilers.__dict__["registered_compilers"][mock_compiler.ext] + for ext in (mock_compiler.ext, second_mock_compiler.ext): + if ext in compilers.__dict__.get("registered_compilers", {}): + del compilers.__dict__["registered_compilers"][ext] def test_compile_source(compilers):