pytest: refactor branches module by introducing named return values (n…

…ear#4970)

Most importantly, add Executables and ABExecutables classes to the
branches module to encapsulate and name values returned by the
prepare_ab_test function.

Issues: near#4956

pytest/lib/branches.py

@@ -3,6 +3,7 @@
 import subprocess
 import sys
 import tempfile
+import typing
 
 import semver
 from configured_logger import logger
@@ -45,7 +46,20 @@ def latest_rc_branch():
     return semver.VersionInfo.parse(releases[0].title).finalize_version()
 
 
-def compile_binary(branch):
+class Executables(typing.NamedTuple):
+    root: pathlib.Path
+    neard: pathlib.Path
+    state_viewer: pathlib.Path
+
+    def node_config(self) -> typing.Dict[str, typing.Any]:
+        return {
+            'local': True,
+            'neard_root': self.root,
+            'binary_name': self.neard.name
+        }
+
+
+def _compile_binary(branch: str) -> Executables:
     """For given branch, compile binary.
 
     Stashes current changes, switches branch and then returns everything back.
@@ -55,28 +69,29 @@ def compile_binary(branch):
     stash_output = subprocess.check_output(['git', 'stash'])
     subprocess.check_output(['git', 'checkout', str(branch)])
     subprocess.check_output(['git', 'pull', 'origin', str(branch)])
-    compile_current(branch)
+    result = _compile_current(branch)
     subprocess.check_output(['git', 'checkout', prev_branch])
     if stash_output != b"No local changes to save\n":
         subprocess.check_output(['git', 'stash', 'pop'])
+    return result
 
 
 def escaped(branch):
     return branch.replace('/', '-')
 
 
-def compile_current(branch=None):
+def _compile_current(branch: str) -> Executables:
     """Compile current branch."""
-    if branch is None:
-        branch = current_branch()
     subprocess.check_call(['cargo', 'build', '-p', 'neard', '--bin', 'neard'])
     subprocess.check_call(['cargo', 'build', '-p', 'near-test-contracts'])
     subprocess.check_call(['cargo', 'build', '-p', 'state-viewer'])
     branch = escaped(branch)
-    os.rename('../target/debug/neard', '../target/debug/neard-%s' % branch)
-    os.rename('../target/debug/state-viewer',
-              '../target/debug/state-viewer-%s' % branch)
-    subprocess.check_call(['git', 'checkout', '../Cargo.lock'])
+    build_dir = pathlib.Path('../target/debug')
+    neard = build_dir / f'neard-{branch}'
+    state_viewer = build_dir / f'state-viewer-{branch}'
+    (build_dir / 'neard').rename(neard)
+    (build_dir / 'state-viewer').rename(state_viewer)
+    return Executables(build_dir, neard, state_viewer)
 
 
 def download_file_if_missing(filename: pathlib.Path, url: str) -> None:
@@ -119,28 +134,40 @@ def download_binary(uname, branch):
     outdir = pathlib.Path('../target/debug')
     basehref = ('https://s3-us-west-1.amazonaws.com/build.nearprotocol.com'
                 f'/nearcore/{uname}/{branch}/')
-    download_file_if_missing(outdir / f'neard-{branch}', basehref + 'neard')
-    download_file_if_missing(outdir / f'state-viewer-{branch}',
-                             basehref + 'state-viewer')
+    neard = outdir / f'neard-{branch}'
+    state_viewer = outdir / f'state-viewer-{branch}'
+    download_file_if_missing(neard, basehref + 'neard')
+    download_file_if_missing(state_viewer, basehref + 'state-viewer')
+    return Executables(outdir, neard, state_viewer)
+
 
+class ABExecutables(typing.NamedTuple):
+    stable: Executables
+    current: Executables
 
-def prepare_ab_test(other_branch):
+
+def prepare_ab_test(stable_branch):
     # Use NEAR_AB_BINARY_EXISTS to avoid rebuild / re-download when testing locally.
     #if not os.environ.get('NEAR_AB_BINARY_EXISTS'):
-    #    compile_current()
+    #    _compile_current(current_branch())
     #    uname = os.uname()[0]
-    #    if other_branch in ['master', 'beta', 'stable'] and uname in ['Linux', 'Darwin']:
-    #        download_binary(uname, other_branch)
+    #    if stable_branch in ['master', 'beta', 'stable'] and uname in ['Linux', 'Darwin']:
+    #        download_binary(uname, stable_branch)
     #    else:
-    uname = os.uname()[0]
-    if not os.getenv('NAYDUCK'):
-        compile_current()
+    is_nayduck = bool(os.getenv('NAYDUCK'))
+
+    if is_nayduck:
+        # On NayDuck the file is fetched from a builder host so there’s no need
+        # to build it.
+        root = pathlib.Path('../target/debug/')
+        current = Executables(root, root / 'neard', root / 'state-viewer')
+    else:
+        current = _compile_current(current_branch())
+
     try:
-        download_binary(uname, other_branch)
+        stable = download_binary(os.uname()[0], stable_branch)
     except Exception:
-        if not os.getenv('NAYDUCK'):
-            compile_binary(str(other_branch))
-        else:
-            logger.critical('RC binary should be downloaded for NayDuck.')
-            sys.exit(1)
-    return '../target/debug/', [other_branch, escaped(current_branch())]
+        if is_nayduck:
+            sys.exit('RC binary should be downloaded for NayDuck.')
+        stable = _compile_binary(str(other_branch))
+    return ABExecutables(stable=stable, current=current)

pytest/lib/cluster.py

@@ -257,7 +257,7 @@ def __init__(self,
         super(LocalNode, self).__init__()
         self.port = port
         self.rpc_port = rpc_port
-        self.near_root = near_root
+        self.near_root = str(near_root)
         self.node_dir = node_dir
         self.binary_name = binary_name or 'neard'
         self.cleaned = False

pytest/tests/sanity/backward_compatible.py

@@ -22,26 +22,20 @@
 def main():
     node_root = get_near_tempdir('backward', clean=True)
     branch = branches.latest_rc_branch()
-    neard_root, (stable_branch,
-                 current_branch) = branches.prepare_ab_test(branch)
+    executables = branches.prepare_ab_test(branch)
 
     # Setup local network.
-    subprocess.call([
-        "%sneard-%s" % (neard_root, stable_branch),
+    subprocess.check_call([
+        executables.stable.neard,
         "--home=%s" % node_root, "testnet", "--v", "2", "--prefix", "test"
     ])
 
     # Run both binaries at the same time.
-    config = {
-        "local": True,
-        'neard_root': neard_root,
-        'binary_name': "neard-%s" % stable_branch
-    }
-    stable_node = cluster.spin_up_node(config, neard_root,
+    config = executables.stable.node_config()
+    stable_node = cluster.spin_up_node(config, executables.stable.root,
                                        str(node_root / 'test0'), 0, None, None)
-    if not os.getenv('NAYDUCK'):
-        config["binary_name"] = "neard-%s" % current_branch
-    current_node = cluster.spin_up_node(config, neard_root,
+    config = executables.current.node_config()
+    current_node = cluster.spin_up_node(config, executables.current.root,
                                         str(node_root / 'test1'), 1,
                                         stable_node.node_key.pk,
                                         stable_node.addr())

pytest/tests/sanity/db_migration.py

@@ -54,34 +54,25 @@ def send_some_tx(node):
 
 
 def main():
-    neard_root, (stable_branch,
-                 current_branch) = branches.prepare_ab_test("master")
+    executables = branches.prepare_ab_test('master')
     node_root = get_near_tempdir('db_migration', clean=True)
 
-    logging.info(f"The near root is {neard_root}...")
+    logging.info(f"The near root is {executables.stable.root}...")
     logging.info(f"The node root is {node_root}...")
 
-    init_command = [
-        "%sneard-%s" % (neard_root, stable_branch),
+    # Init local node
+    subprocess.call((
+        executables.stable.neard,
         "--home=%s" % node_root,
         "init",
         "--fast",
-    ]
-
-    # Init local node
-    subprocess.call(init_command)
+    ))
 
     # Run stable node for few blocks.
-    config = {
-        "local": True,
-        'neard_root': neard_root,
-        'binary_name': "neard-%s" % stable_branch
-    }
-
     logging.info("Starting the stable node...")
-
-    node = cluster.spin_up_node(config, neard_root, str(node_root), 0, None,
-                                None)
+    config = executables.stable.node_config()
+    node = cluster.spin_up_node(config, executables.stable.root, str(node_root),
+                                0, None, None)
 
     logging.info("Running the stable node...")
     wait_for_blocks_or_timeout(node, 20, 100)
@@ -95,9 +86,8 @@ def main():
         "Stable node has produced blocks... Stopping the stable node... ")
 
     # Run new node and verify it runs for a few more blocks.
-    config["binary_name"] = "neard-%s" % current_branch
-
     logging.info("Starting the current node...")
+    config = executables.current.node_config()
     node.binary_name = config['binary_name']
     node.start(node.node_key.pk, node.addr())
 

pytest/tests/sanity/state_migration.py

@@ -24,9 +24,7 @@
 
 def main():
     node_root = get_near_tempdir('state_migration', clean=True)
-
-    near_root, (stable_branch,
-                current_branch) = branches.prepare_ab_test("beta")
+    executables = branches.prepare_ab_test('beta')
 
     # Run stable node for few blocks.
     subprocess.call([

pytest/tests/sanity/upgradable.py

@@ -25,18 +25,13 @@ def main():
     node_root = get_near_tempdir('upgradable', clean=True)
     branch = branches.latest_rc_branch()
     logger.info(f"Latest rc release branch is {branch}")
-    neard_root, (stable_branch,
-                 current_branch) = branches.prepare_ab_test(branch)
+    executables = branches.prepare_ab_test(branch)
 
     # Setup local network.
-    logger.info([
-        "%sneard-%s" % (neard_root, stable_branch),
-        "--home=%s" % node_root, "testnet", "--v", "4", "--prefix", "test"
-    ])
-    subprocess.call([
-        "%sneard-%s" % (neard_root, stable_branch),
-        "--home=%s" % node_root, "testnet", "--v", "4", "--prefix", "test"
-    ])
+    cmd = (executables.stable.neard, "--home=%s" % node_root, "testnet", "--v",
+           "4", "--prefix", "test")
+    logger.info(' '.join(str(arg) for arg in cmd))
+    subprocess.check_call(cmd)
     genesis_config_changes = [("epoch_length", 20),
                               ("num_block_producer_seats", 10),
                               ("num_block_producer_seats_per_shard", [10]),
@@ -48,22 +43,18 @@ def main():
         cluster.apply_genesis_changes(node_dir, genesis_config_changes)
 
     # Start 3 stable nodes and one current node.
-    config = {
-        "local": True,
-        'neard_root': neard_root,
-        'binary_name': "neard-%s" % stable_branch
-    }
+    config = executables.stable.node_config()
     nodes = [
-        cluster.spin_up_node(config, neard_root, node_dirs[0], 0, None, None)
+        cluster.spin_up_node(config, executables.stable.root, node_dirs[0], 0,
+                             None, None)
     ]
     for i in range(1, 3):
         nodes.append(
-            cluster.spin_up_node(config, neard_root, node_dirs[i], i,
-                                 nodes[0].node_key.pk, nodes[0].addr()))
-    if not os.getenv('NAYDUCK'):
-        config["binary_name"] = "neard-%s" % current_branch
+            cluster.spin_up_node(config, executables.stable.root, node_dirs[i],
+                                 i, nodes[0].node_key.pk, nodes[0].addr()))
+    config = executables.current.node_config()
     nodes.append(
-        cluster.spin_up_node(config, neard_root, node_dirs[3], 3,
+        cluster.spin_up_node(config, executables.current.root, node_dirs[3], 3,
                              nodes[0].node_key.pk, nodes[0].addr()))
 
     time.sleep(2)