From 17a0e49e48ef12a562676e0bac8a82e4638c960a Mon Sep 17 00:00:00 2001 From: iamdefinitelyahuman Date: Sun, 12 May 2019 23:39:53 +0300 Subject: [PATCH 1/7] reset instead of revert when blockNumber is 0 --- brownie/network/rpc.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/brownie/network/rpc.py b/brownie/network/rpc.py index 0f029aa65..70181f358 100644 --- a/brownie/network/rpc.py +++ b/brownie/network/rpc.py @@ -145,7 +145,10 @@ def _revert(self, id_): id_ = self._snap() self.sleep(0) for i in self._objects: - i._revert() + if web3.eth.blockNumber == 0: + i._reset() + else: + i._revert() return id_ def _reset(self): From 8691f107f50e0f1adeb03484ff6bade419b7bf16 Mon Sep 17 00:00:00 2001 From: iamdefinitelyahuman Date: Mon, 13 May 2019 01:37:08 +0300 Subject: [PATCH 2/7] evaluate coverage after each test, avoid redunant tx evaluation --- brownie/cli/test.py | 54 ++++++++++++++++++++-------------------- brownie/test/coverage.py | 30 +++++++++++++--------- 2 files changed, 45 insertions(+), 39 deletions(-) diff --git a/brownie/cli/test.py b/brownie/cli/test.py index 0161ad54f..cdfaa9b11 100644 --- a/brownie/cli/test.py +++ b/brownie/cli/test.py @@ -13,7 +13,8 @@ from brownie.cli.utils import color from brownie.test.coverage import ( analyze_coverage, - merge_coverage, + merge_coverage_eval, + merge_coverage_files, generate_report ) from brownie.exceptions import ExpectedFailing @@ -33,6 +34,8 @@ WARN = "{0[error]}WARNING{0}: ".format(color) ERROR = "{0[error]}ERROR{0}: ".format(color) +history = TxHistory() + __doc__ = """Usage: brownie test [] [] [options] Arguments: @@ -57,7 +60,6 @@ def main(): ARGV._update_from_args(args) if ARGV['coverage']: ARGV['always_transact'] = True - history = TxHistory() history._revert_lock = True test_paths = get_test_paths(args['']) @@ -178,7 +180,7 @@ def run_test_modules(test_data, save): print("\n\nTest execution has been terminated by KeyboardInterrupt.") sys.exit() finally: - print("\nTotal runtime: {:.4}s".format(time.time() - start_time)) + print("\nTotal runtime: {:.4f}s".format(time.time() - start_time)) if traceback_info: print("{0}{1} test{2} failed.".format( WARN, @@ -196,7 +198,6 @@ def run_test(module, network, test_names): if type(CONFIG['test']['gas_limit']) is int: network.gas_limit(CONFIG['test']['gas_limit']) - traceback_info = [] if 'setup' in test_names: test_names.remove('setup') fn, default_args = _get_fn(module, 'setup') @@ -206,38 +207,38 @@ def run_test(module, network, test_names): ): return [], {} p = TestPrinter(module.__file__, 0, len(test_names)) - traceback_info += run_test_method(fn, default_args, p) - if traceback_info: - return traceback_info, {} + tb, coverage_eval = run_test_method(fn, default_args, {}, p) + if tb: + return tb, {} else: p = TestPrinter(module.__file__, 1, len(test_names)) default_args = FalseyDict() + coverage_eval = {} network.rpc.snapshot() + traceback_info = [] for t in test_names: network.rpc.revert() fn, fn_args = _get_fn(module, t) args = default_args.copy() args.update(fn_args) - traceback_info += run_test_method(fn, args, p) - if traceback_info and traceback_info[-1][2] == ReadTimeout: + tb, coverage_eval = run_test_method(fn, args, coverage_eval, p) + traceback_info += tb + if tb and tb[0][2] == ReadTimeout: print(WARN+"RPC crashed, terminating test") network.rpc.kill(False) network.rpc.launch(CONFIG['active_network']['test-rpc']) break - coverage_eval = {} - if not traceback_info and ARGV['coverage']: - p.start("Evaluating test coverage") - coverage_eval = analyze_coverage(TxHistory().copy()) - p.stop() + if traceback_info and ARGV['coverage']: + coverage_eval = {} p.finish() return traceback_info, coverage_eval -def run_test_method(fn, args, p): +def run_test_method(fn, args, coverage_eval, p): desc = fn.__doc__ or fn.__name__ if args['skip'] is True or (args['skip'] == "coverage" and ARGV['coverage']): p.skip(desc) - return [] + return [], coverage_eval p.start(desc) try: if ARGV['coverage'] and 'always_transact' in args: @@ -245,28 +246,27 @@ def run_test_method(fn, args, p): fn() if ARGV['coverage']: ARGV['always_transact'] = True + coverage_eval = merge_coverage_eval( + coverage_eval, + analyze_coverage(history.copy()) + ) + history.clear() if args['pending']: raise ExpectedFailing("Test was expected to fail") p.stop() - return [] + return [], coverage_eval except Exception as e: p.stop(e, args['pending']) if type(e) != ExpectedFailing and args['pending']: - return [] + return [], coverage_eval path = Path(sys.modules[fn.__module__].__file__).relative_to(CONFIG['folders']['project']) path = "{0[module]}{1}.{0[callable]}{2}{0}".format(color, str(path)[:-3], fn.__name__) - return [( - path, - color.format_tb( - sys.exc_info(), - sys.modules[fn.__module__].__file__, - ), - type(e) - )] + tb = color.format_tb(sys.exc_info(), sys.modules[fn.__module__].__file__) + return [(path, tb, type(e))], coverage_eval def display_report(coverage_files, save): - coverage_eval = merge_coverage(coverage_files) + coverage_eval = merge_coverage_files(coverage_files) report = generate_report(coverage_eval) print("\nCoverage analysis:") for name in coverage_eval: diff --git a/brownie/test/coverage.py b/brownie/test/coverage.py index 1380aea4d..2d6fd32f1 100644 --- a/brownie/test/coverage.py +++ b/brownie/test/coverage.py @@ -26,7 +26,7 @@ def analyze_coverage(history): pc = t['pc'] name = t['contractName'] path = t['source']['filename'] - if not name or not path: + if not name or not path or name not in build: continue # prevent repeated requests to build object @@ -72,16 +72,10 @@ def analyze_coverage(history): return _calculate_pct(coverage_eval) -def merge_coverage(coverage_files): - '''Given a list of coverage evaluation json file paths, returns an aggregated - coverage evaluation dict. - ''' - merged_eval = {} - for filename in coverage_files: - path = Path(filename) - if not path.exists(): - continue - coverage = json.load(path.open())['coverage'] +def merge_coverage_eval(*coverage_eval): + '''Given a list of coverage evaluation dicts, returns an aggregated evaluation dict.''' + merged_eval = coverage_eval[0] + for coverage in coverage_eval[1:]: for contract_name in list(coverage): del coverage[contract_name]['pct'] if contract_name not in merged_eval: @@ -106,6 +100,17 @@ def merge_coverage(coverage_files): return _calculate_pct(merged_eval) +def merge_coverage_files(coverage_files): + '''Given a list of coverage evaluation file paths, returns an aggregated evaluation dict.''' + coverage_eval = [] + for filename in coverage_files: + path = Path(filename) + if not path.exists(): + continue + coverage_eval.append(json.load(path.open())['coverage']) + return merge_coverage_eval(*coverage_eval) + + def _calculate_pct(coverage_eval): '''Internal method to calculate coverage percentages''' for name in coverage_eval: @@ -121,7 +126,8 @@ def _calculate_pct(coverage_eval): contract_count += build[name]['coverageMapTotals'][fn_name] result[fn_name] = {'pct': result[fn_name]['pct']} continue - result = result[fn_name] + result = dict((k, list(v) if type(v) is set else v) for k, v in result[fn_name].items()) + coverage_eval[name][path][fn_name] = result count = 0 maps = coverage_map[path][fn_name] for idx, item in enumerate(maps): From d2119367fe902cc456d970046cda00c52291092d Mon Sep 17 00:00:00 2001 From: iamdefinitelyahuman Date: Mon, 13 May 2019 01:38:47 +0300 Subject: [PATCH 3/7] return error message when RPC call fails --- brownie/exceptions.py | 4 ++++ brownie/network/rpc.py | 9 +++++++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/brownie/exceptions.py b/brownie/exceptions.py index aceb1528c..0ec455cf5 100644 --- a/brownie/exceptions.py +++ b/brownie/exceptions.py @@ -39,6 +39,10 @@ def __init__(self, cmd, proc, uri): super().__init__("Able to launch RPC client, but unable to connect.", cmd, proc, uri) +class RPCRequestError(Exception): + pass + + class VirtualMachineError(Exception): '''Raised when a call to a contract causes an EVM exception. diff --git a/brownie/network/rpc.py b/brownie/network/rpc.py index 70181f358..f40e36000 100644 --- a/brownie/network/rpc.py +++ b/brownie/network/rpc.py @@ -9,7 +9,7 @@ from .web3 import Web3 from brownie.types.types import _Singleton -from brownie.exceptions import RPCProcessError, RPCConnectionError +from brownie.exceptions import RPCProcessError, RPCConnectionError, RPCRequestError web3 = Web3() @@ -131,9 +131,12 @@ def _request(self, *args): if not self.is_active(): raise SystemError("RPC is not active.") try: - return web3.providers[0].make_request(*args)['result'] + response = web3.providers[0].make_request(*args) + if 'result' in response: + return response['result'] except IndexError: raise RPCConnectionError("Web3 is not connected.") + raise RPCRequestError(response['error']['message']) def _snap(self): return self._request("evm_snapshot", []) @@ -204,12 +207,14 @@ def revert(self): '''Reverts the EVM to the most recently taken snapshot.''' if not self._snapshot_id: raise ValueError("No snapshot set") + self._internal_id = None self._snapshot_id = self._revert(self._snapshot_id) return "Block height reverted to {}".format(web3.eth.blockNumber) def reset(self): '''Reverts the EVM to the genesis state.''' self._snapshot_id = None + self._internal_id = None self._reset_id = self._revert(self._reset_id) return "Block height reset to 0" From 16e66d1c0c865407540170ed24164bc74beb1d32 Mon Sep 17 00:00:00 2001 From: iamdefinitelyahuman Date: Mon, 13 May 2019 01:39:16 +0300 Subject: [PATCH 4/7] bugfixes --- brownie/cli/test.py | 9 ++++++--- brownie/network/contract.py | 9 ++++++--- brownie/network/history.py | 3 +++ brownie/project/build.py | 9 ++++++--- brownie/project/sources.py | 1 + brownie/test/coverage.py | 2 +- 6 files changed, 23 insertions(+), 10 deletions(-) diff --git a/brownie/cli/test.py b/brownie/cli/test.py index cdfaa9b11..3afc7b0bf 100644 --- a/brownie/cli/test.py +++ b/brownie/cli/test.py @@ -111,8 +111,11 @@ def get_test_data(test_paths): coverage_files = [] test_data = [] project = Path(CONFIG['folders']['project']) + build_path = project.joinpath('build/coverage') for path in test_paths: - coverage_json = project.joinpath("build/coverage/"+path.stem+".json") + path = path.relative_to(project) + coverage_json = build_path.joinpath(path.parent.relative_to('tests')) + coverage_json = coverage_json.joinpath(path.stem+".json") coverage_files.append(coverage_json) if coverage_json.exists(): coverage_eval = json.load(coverage_json.open())['coverage'] @@ -122,7 +125,7 @@ def get_test_data(test_paths): coverage_eval = {} for p in list(coverage_json.parents)[::-1]: p.mkdir(exist_ok=True) - module_name = str(path.relative_to(project))[:-3].replace(os.sep, '.') + module_name = str(path)[:-3].replace(os.sep, '.') module = importlib.import_module(module_name) test_names = re.findall(r'\ndef[\s ]{1,}([^_]\w*)[\s ]*\([^)]*\)', path.open().read()) if not test_names: @@ -131,7 +134,7 @@ def get_test_data(test_paths): duplicates = set([i for i in test_names if test_names.count(i) > 1]) if duplicates: raise ValueError("{} contains multiple test methods of the same name: {}".format( - path.relative_to(project), + path, ", ".join(duplicates) )) if 'setup' in test_names: diff --git a/brownie/network/contract.py b/brownie/network/contract.py index 8bfaca0d6..600663348 100644 --- a/brownie/network/contract.py +++ b/brownie/network/contract.py @@ -284,7 +284,7 @@ def call(self, *args): return format_output(result[0]) return KwargTuple(result, self.abi) - def transact(self, *args): + def transact(self, *args, _rpc_clear=True): '''Broadcasts a transaction that calls this contract method. Args: @@ -299,7 +299,8 @@ def transact(self, *args): "Contract has no owner, you must supply a tx dict" " with a 'from' field as the last argument." ) - rpc._internal_clear() + if _rpc_clear: + rpc._internal_clear() return tx['from'].transfer( self._address, tx['value'], @@ -368,7 +369,9 @@ def __call__(self, *args): Contract method return value(s).''' if ARGV['always_transact']: rpc._internal_snap() - tx = self.transact(*args, {'gas_price': 0}) + args, tx = _get_tx(self._owner, args) + tx['gas_price'] = 0 + tx = self.transact(*args, tx, _rpc_clear=False) if tx.modified_state: rpc._internal_revert() return tx.return_value diff --git a/brownie/network/history.py b/brownie/network/history.py index b79a0079c..1d8533cd2 100644 --- a/brownie/network/history.py +++ b/brownie/network/history.py @@ -51,6 +51,9 @@ def _console_repr(self): def _add_tx(self, tx): self._list.append(tx) + def clear(self): + self._list.clear() + def copy(self): '''Returns a shallow copy of the object as a list''' return self._list.copy() diff --git a/brownie/project/build.py b/brownie/project/build.py index 44fc17996..b09149329 100644 --- a/brownie/project/build.py +++ b/brownie/project/build.py @@ -47,6 +47,12 @@ def __init__(self): self._build = {} self._path = None + def __getitem__(self, contract_name): + return self._build[contract_name.replace('.json', '')] + + def __contains__(self, contract_name): + return contract_name.replace('.json', '') in self._build + def _load(self): self. _path = Path(CONFIG['folders']['project']).joinpath('build/contracts') # check build paths @@ -119,9 +125,6 @@ def _check_coverage_hashes(self): coverage_json.unlink() break - def __getitem__(self, contract_name): - return self._build[contract_name.replace('.json', '')] - def items(self): return self._build.items() diff --git a/brownie/project/sources.py b/brownie/project/sources.py index 63d1ec9a7..03846e9dc 100644 --- a/brownie/project/sources.py +++ b/brownie/project/sources.py @@ -134,6 +134,7 @@ def inheritance_map(self): def add_source(self, source): path = "".format(self._string_iter) self._source[path] = source + self._remove_comments(path) self._get_contract_data(path) self._string_iter += 1 return path diff --git a/brownie/test/coverage.py b/brownie/test/coverage.py index 2d6fd32f1..496b7f8c9 100644 --- a/brownie/test/coverage.py +++ b/brownie/test/coverage.py @@ -208,7 +208,7 @@ def _evaluate_branch(path, ln): after = next((i for i in after if i != ")"), after[0])[0] if ( (before[-2:] == "if" and after == "|") or - (before[:7] == "require" and after in (")", "|")) + (before[:7] == "require" and after in (")", "|", ",")) ): return True return False From ee50389dc01f3feb97897edba3c6b3c2ec4efc15 Mon Sep 17 00:00:00 2001 From: iamdefinitelyahuman Date: Mon, 13 May 2019 03:51:01 +0300 Subject: [PATCH 5/7] add broadcast_reverting_tx test value override, add warning, update docs --- brownie/cli/test.py | 8 ++++++-- brownie/data/config.json | 3 ++- brownie/network/contract.py | 2 +- docs/config.rst | 6 ++++-- docs/tests.rst | 9 +++++++++ 5 files changed, 22 insertions(+), 6 deletions(-) diff --git a/brownie/cli/test.py b/brownie/cli/test.py index 3afc7b0bf..4523a32d3 100644 --- a/brownie/cli/test.py +++ b/brownie/cli/test.py @@ -58,6 +58,8 @@ def main(): args = docopt(__doc__) ARGV._update_from_args(args) +# if type(CONFIG['test']['gas_limit']) is int: +# network.gas_limit(CONFIG['test']['gas_limit']) if ARGV['coverage']: ARGV['always_transact'] = True history._revert_lock = True @@ -150,6 +152,10 @@ def run_test_modules(test_data, save): count = sum([len([x for x in i[3] if x != "setup"]) for i in test_data]) print("Running {} tests across {} modules.".format(count, len(test_data))) network.connect(ARGV['network']) + for key in ('broadcast_reverting_tx', 'gas_limit'): + CONFIG['active_network'][key] = CONFIG['test'][key] + if not CONFIG['active_network']['broadcast_reverting_tx']: + print("{0[error]}WARNING{0}: Reverting transactions will NOT be broadcasted.".format(color)) traceback_info = [] start_time = time.time() try: @@ -198,8 +204,6 @@ def run_test_modules(test_data, save): def run_test(module, network, test_names): network.rpc.reset() - if type(CONFIG['test']['gas_limit']) is int: - network.gas_limit(CONFIG['test']['gas_limit']) if 'setup' in test_names: test_names.remove('setup') diff --git a/brownie/data/config.json b/brownie/data/config.json index 9fe343c91..24e24528f 100644 --- a/brownie/data/config.json +++ b/brownie/data/config.json @@ -17,7 +17,8 @@ }, "test":{ "gas_limit": 6721975, - "default_contract_owner": false + "default_contract_owner": false, + "broadcast_reverting_tx": true }, "solc":{ "optimize": true, diff --git a/brownie/network/contract.py b/brownie/network/contract.py index 600663348..481449b49 100644 --- a/brownie/network/contract.py +++ b/brownie/network/contract.py @@ -332,7 +332,7 @@ class ContractTx(_ContractMethod): def __init__(self, fn, abi, name, owner): if ( - ARGV['cli'] != "console" and not + ARGV['cli'] == "test" and not CONFIG['test']['default_contract_owner'] ): owner = None diff --git a/docs/config.rst b/docs/config.rst index 42c2fe0bf..918a0b713 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -43,9 +43,11 @@ The following settings are available: .. py:attribute:: test - Properties that affect only affect Brownie's configuration when running scripts and tests. See Test :ref:`test_settings` for detailed information on the effects and implications of these settings. + Properties that affect only affect Brownie's configuration when running tests. See Test :ref:`test_settings` for detailed information on the effects and implications of these settings. - * ``gas_limit``: If set to an integer, this value will over-ride the default gas limit setting when running tests. + * ``gas_limit``: Replaces the default network gas limit. + + * ``broadcast_reverting_tx``: Replaces the default network setting for broadcasting reverting transactions. * ``default_contract_owner``: If ``false``, deployed contracts will not remember the account that they were created by and you will have to supply a ``from`` kwarg for every contract transaction. diff --git a/docs/tests.rst b/docs/tests.rst index 682334ecc..ff8281fd7 100644 --- a/docs/tests.rst +++ b/docs/tests.rst @@ -192,10 +192,19 @@ The following test configuration settings are available in ``brownie-config.json { "test": { "gas_limit": 6721975, + "broadcast_reverting_tx": true, "default_contract_owner": false } } +.. py:attribute:: gas_limit + + Replaces the default network gas limit. + +.. py:attribute:: broadcast_reverting_tx + + Replaces the default network setting for broadcasting transactions that would revert. + .. py:attribute:: default_contract_owner If ``True``, calls to contract transactions that do not specify a sender are broadcast from the same address that deployed the contract. From 5053bcf4a82df3c1c57df3933abfdfd8617b9af6 Mon Sep 17 00:00:00 2001 From: iamdefinitelyahuman Date: Mon, 13 May 2019 04:24:19 +0300 Subject: [PATCH 6/7] sort coverage report output by contract --- brownie/cli/test.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/brownie/cli/test.py b/brownie/cli/test.py index 4523a32d3..650b02ffe 100644 --- a/brownie/cli/test.py +++ b/brownie/cli/test.py @@ -276,7 +276,7 @@ def display_report(coverage_files, save): coverage_eval = merge_coverage_files(coverage_files) report = generate_report(coverage_eval) print("\nCoverage analysis:") - for name in coverage_eval: + for name in sorted(coverage_eval): pct = coverage_eval[name].pop('pct') c = color(next(i[1] for i in COVERAGE_COLORS if pct <= i[0])) print("\n contract: {0[contract]}{1}{0} - {2}{3:.1%}{0}".format(color, name, c, pct)) From a2467e3fc48c0a56a8345b7808a7bc63118e589b Mon Sep 17 00:00:00 2001 From: iamdefinitelyahuman Date: Mon, 13 May 2019 04:26:07 +0300 Subject: [PATCH 7/7] raise ValueError on unknown function --- brownie/project/sources.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/brownie/project/sources.py b/brownie/project/sources.py index 03846e9dc..8f60cc70b 100644 --- a/brownie/project/sources.py +++ b/brownie/project/sources.py @@ -121,12 +121,15 @@ def get_fn(self, name, start, stop): return False if stop > offset[2] else offset[0] def get_fn_offset(self, name, fn_name): - if name not in self._data: - name = next( - k for k, v in self._data.items() if v['sourcePath'] == str(name) and - fn_name in [i[0] for i in v['fn_offsets']] - ) - return next(i for i in self._data[name]['fn_offsets'] if i[0] == fn_name)[1:3] + try: + if name not in self._data: + name = next( + k for k, v in self._data.items() if v['sourcePath'] == str(name) and + fn_name in [i[0] for i in v['fn_offsets']] + ) + return next(i for i in self._data[name]['fn_offsets'] if i[0] == fn_name)[1:3] + except StopIteration: + raise ValueError("Unknown function '{}' in contract {}".format(fn_name, name)) def inheritance_map(self): return dict((k, v['inherited'].copy()) for k, v in self._data.items())