Problem (fix #154 #156): can't use pytest cli directly in nix-shell (#…

…157)

Solution:
- seperate dev env and integration test env in nix-shell
  - minimize dependencies running in ci
  - manipulate PYTHONPATH to make pystarport editable in dev env
- flake8 don't check naming conventions, add the `pep8-naming`
  plugin in nix-shell.
- use pytest's builtin tmp_path fixture to manage tmp directories.
- add entrypoint `make lintpy` to run python linter

.github/workflows/nix.yml

@@ -23,9 +23,7 @@ jobs:
         nix-env -iA cachix -f https://cachix.org/api/v1/install
         cachix use crypto-com
       if: github.ref != 'refs/heads/master'
-    - run: nix run nixpkgs.python3Packages.flake8 -c flake8 --max-line-length 88 --extend-ignore E203 integration_tests pystarport
-    - run: nix run nixpkgs.python3Packages.black -c black --check --diff integration_tests pystarport
-    - run: nix-shell --run "isort -c --diff -rc integration_tests pystarport"
+    - run: nix-shell integration_tests/shell.nix --run "make lintpy"
     - run: make nix-integration-test
     - name: Upload coverage report
       uses: codecov/codecov-action@v1

Makefile

@@ -66,6 +66,11 @@ lint:
 	@golangci-lint run
 	@go mod verify
 
+lintpy:
+	@flake8
+	@black --check --diff .
+	@isort -c --diff -rc .
+
 test-sim-nondeterminism:
 	@echo "Running non-determinism test..."
 	@go test -mod=readonly $(SIMAPP) -run TestAppStateDeterminism -Enabled=true \
@@ -115,7 +120,7 @@ clean-docker-compose: localnet-stop
 ###############################################################################
 # nix installation: https://nixos.org/download.html
 nix-integration-test:
-	nix-shell --run "python -mpytest -v -n 3 --dist loadscope integration_tests"
+	nix-shell integration_tests/shell.nix --run "pytest -v -n 3 --dist loadscope"
 
 nix-build-%: check-os
 	@if [ -e ~/.nix/remote-build-env ]; then \

docker.nix

@@ -1,33 +1,18 @@
-{ system ? "x86_64-linux" }:
+{ system ? "x86_64-linux", pkgs ? import <nixpkgs> { inherit system; } }:
 
 let
-  pkgs = import <nixpkgs> { inherit system; };
-
   chaind = import ./. { inherit pkgs; };
   pystarport = import ./pystarport { inherit pkgs; };
-
+in {
   chaindImage =
-    pkgs.dockerTools.buildImage {
+    pkgs.dockerTools.buildLayeredImage {
       name = "crypto-com/chain-maind";
-      tag = chaind.version;
-
-      contents = [ chaind ];
-
-      config = {
-        Cmd = [ "${chaind}/bin/chain-maind" ];
-      };
+      config.Entrypoint = [ "${chaind}/bin/chain-maind" ];
     };
 
   pystarportImage =
-    pkgs.dockerTools.buildImage {
+    pkgs.dockerTools.buildLayeredImage {
       name = "crypto-com/chain-main-pystarport";
-      tag = pystarport.version;
-
-      contents = [ chaind pystarport ];
-
-      config = {
-        Cmd = [ "${pystarport}/bin/pystarport" ];
-      };
+      config.Entrypoint = [ "${pystarport}/bin/pystarport" ];
     };
-
-in { inherit chaindImage pystarportImage; }
+  }

integration_tests/README.md

@@ -1,8 +1,13 @@
-We use [pytest](https://docs.pytest.org/) to run integration tests. pytest discover test cases
+## Pytest
+
+To run integration tests, just run `pytest` in root directory of repo, We use [pytest](https://docs.pytest.org/) to run
+integration tests. pytest discover test cases
 [automatically](https://docs.pytest.org/en/3.0.6/goodpractices.html#conventions-for-python-test-discovery). notably the
 python source files whose name starts with `test_` is treated as test modules, and the functions in them whose name
 starts with `test_` will be executed as test cases.
 
+### Fixture
+
 We use [pytest.fixture]() to setup/teardown testnet cluster, fixture can be defined at different scopes, we use a
 session scoped fixture for default cluster, we can override it with a module scoped one at test modules which require
 special testnet setup.
@@ -11,8 +16,11 @@ To use a custom cluster for your test case, just create module scoped fixture in
 this](https://github.com/crypto-com/chain-main/blob/cb0005fd64250e08e4f758138db6a11fcec71d03/integration_tests/test_slashing.py#L17).
 you can put the custom cluster config file into the `integration_tests/configs` directory.
 
-To write test case which depend on the default cluster, one only need to accept a `cluster` parameter, then you get
-access to the [`ClusterCLI`](../pystarport/pystarport/cluster.py#L38) object automagically.
+To write test case which depend on the default cluster, one only need to create a test function which accept the
+`cluster` parameter, then you get access to the [`ClusterCLI`](../pystarport/pystarport/cluster.py#L38) object
+automagically.
+
+### Concurrency
 
 We use [python-xdist](https://pypi.org/project/pytest-xdist/) to execute test cases in multi-processess in parallel, the
 implications are:
@@ -25,9 +33,14 @@ of this when choosing `base_port` when overriding the default cluster.
 
   > [pystarport's port rules](../pystarport/README.md#port-rules)
 
-We can use `@pytest.mark.slow` to mark slow test cases (like slashing test which need to sleep quit long time to wait
-for blockchain events), we can pass command line argument `-m 'not slow'` to pytest to skip these slow test cases for
-faster development.
+### Markers
+
+We can use [markers](https://docs.pytest.org/en/stable/example/markers.html) to mark test cases, currently we  use
+`slow` to mark test cases that runs slow (like slashing test which need to sleep quit long time to wait
+for blockchain events), we select or unselect test cases with markers. For example, passing `-m 'not slow'` to pytest
+can skip the slow test cases, useful for development.
+
+### Cluster api
 
 `cluster` is an instance of
 [`ClusterCLI`](https://github.com/crypto-com/chain-main/blob/master/pystarport/pystarport/cluster.py#L21), which is used
@@ -48,4 +61,16 @@ cluster.address('validator', i=2, bech='val')
 cluster.transfer('from addr', 'to addr', '1cro', i=2)
 ```
 
-[pystarport doc](../pystarport/README.md)
+### Temparary directory
+
+We use the [`tmp_path_factory`](https://docs.pytest.org/en/stable/tmpdir.html#the-tmp-path-factory-fixture) to create
+the data directory of test chain, pytest will take care of it's cleanup, particularly:
+
+- it's not cleaned up immediately after test runs, so you can investigate it later in development
+- it only keeps the directories of recent 3 test runs
+
+For example, you can find the data directory of default cluster in most recent run here: `/tmp/pytest-of-$(whoami)/pytest-current/chaintestcurrent/`.
+
+## pystarport
+
+The integration tests use [pystarport](../pystarport/README.md) to setup chain environment.

integration_tests/conftest.py

@@ -21,14 +21,15 @@ def pytest_addoption(parser):
 
 
 @pytest.fixture(scope="session")
-def cluster(worker_id, pytestconfig):
+def cluster(worker_id, pytestconfig, tmp_path_factory):
     "default cluster fixture"
     match = re.search(r"\d+", worker_id)
     worker_id = int(match[0]) if match is not None else 0
     base_port = (100 + worker_id) * 100
     yield from cluster_fixture(
         Path(__file__).parent / "configs/default.yaml",
         base_port,
+        tmp_path_factory,
         quiet=pytestconfig.getoption("supervisord-quiet"),
     )
 
@@ -37,7 +38,7 @@ def cluster(worker_id, pytestconfig):
 def suspend_capture(pytestconfig):
     "used for pause in testing"
 
-    class suspend_guard:
+    class SuspendGuard:
         def __init__(self):
             self.capmanager = pytestconfig.pluginmanager.getplugin("capturemanager")
 
@@ -47,4 +48,4 @@ def __enter__(self):
         def __exit__(self, _1, _2, _3):
             self.capmanager.resume_global_capture()
 
-    yield suspend_guard()
+    yield SuspendGuard()

integration_tests/shell.nix

@@ -0,0 +1,20 @@
+{ pkgs ? import <nixpkgs> {} }:
+with pkgs;
+let
+  chain = import ../. { inherit pkgs; };
+  pystarport = import ../pystarport { inherit pkgs; };
+in
+  mkShell {
+    buildInputs = [
+      chain.instrumented
+      pystarport
+      (with python3Packages; [
+        pytest
+        pytest_xdist
+        flake8
+        black
+        isort
+        pep8-naming
+      ])
+    ];
+  }

integration_tests/test_multisig.py

@@ -1,21 +1,14 @@
 import json
-import tempfile
-from pathlib import Path
 
 from .utils import wait_for_new_blocks
 
 
-def test_multi_signature(cluster):
-    with tempfile.TemporaryDirectory() as tmpdir:
-        do_test_multi_signature(cluster, Path(tmpdir))
-
-
-def do_test_multi_signature(cluster, tmpdir):
+def test_multi_signature(cluster, tmp_path):
     # prepare files
-    m_txt = tmpdir / "m.txt"
-    p1_txt = tmpdir / "p1.txt"
-    p2_txt = tmpdir / "p2.txt"
-    tx_txt = tmpdir / "tx.txt"
+    m_txt = tmp_path / "m.txt"
+    p1_txt = tmp_path / "p1.txt"
+    p2_txt = tmp_path / "p2.txt"
+    tx_txt = tmp_path / "tx.txt"
 
     # make multi-sig
     cluster.make_multisig("multitest1", "signer1", "signer2")

integration_tests/test_slashing.py

@@ -14,11 +14,12 @@
 
 # use custom cluster, use an unique base port
 @pytest.fixture(scope="module")
-def cluster(pytestconfig):
+def cluster(pytestconfig, tmp_path_factory):
     "override cluster fixture for this test module"
     yield from cluster_fixture(
         Path(__file__).parent / "configs/slashing.yaml",
         26700,
+        tmp_path_factory,
         quiet=pytestconfig.getoption("supervisord-quiet"),
     )
 

integration_tests/utils.py

@@ -1,6 +1,6 @@
+import re
 import socket
 import sys
-import tempfile
 import time
 from pathlib import Path
 
@@ -56,46 +56,47 @@ def wait_for_port(port, host="127.0.0.1", timeout=40.0):
                 ) from ex
 
 
-def cluster_fixture(config_path, base_port, quiet=False):
+def cluster_fixture(config_path, base_port, tmp_path_factory, quiet=False):
     config = yaml.safe_load(open(config_path))
-    with tempfile.TemporaryDirectory(suffix=config["chain_id"]) as tmpdir:
-        print("init cluster at", tmpdir, ", base port:", base_port)
-        data = Path(tmpdir)
-        cluster.init_cluster(data, config, base_port)
-
-        # replace the first node with the instrumented binary
-        ini = data / "tasks.ini"
-        ini.write_text(
-            ini.read_text().replace(
-                "chain-maind",
-                f"chain-maind-inst -test.coverprofile={data}/coverage.out",
-                1,
-            )
+    data = tmp_path_factory.mktemp(config["chain_id"])
+    print("init cluster at", data, ", base port:", base_port)
+    cluster.init_cluster(data, config, base_port)
+
+    # replace the first node with the instrumented binary
+    ini = data / "tasks.ini"
+    ini.write_text(
+        re.sub(
+            r"^command = .*/chain-maind",
+            "command = chain-maind-inst -test.coverprofile=%(here)s/coverage.out",
+            ini.read_text(),
+            count=1,
+            flags=re.M,
         )
-        begin = time.time()
-
-        supervisord = cluster.start_cluster(data, quiet=quiet)
-        # wait for first node rpc port available before start testing
-        wait_for_port(rpc_port(base_port, 0))
-        cli = cluster.ClusterCLI(data)
-        # wait for first block generated before start testing
-        wait_for_block(cli, 1)
-
-        yield cli
-
-        duration = time.time() - begin
-        # wait for server startup complete to generate the coverage report
-        if duration < 15:
-            time.sleep(15 - duration)
-
-        supervisord.terminate()
-        supervisord.wait()
-
-        # collect the coverage results
-        txt = (data / "coverage.out").read_text()
-        merged = Path("coverage.txt")
-        if merged.exists():
-            assert txt.startswith("mode: set")
-            txt = txt[10:]
-        with merged.open("a") as f:
-            f.write(txt)
+    )
+    begin = time.time()
+
+    supervisord = cluster.start_cluster(data, quiet=quiet)
+    # wait for first node rpc port available before start testing
+    wait_for_port(rpc_port(base_port, 0))
+    cli = cluster.ClusterCLI(data)
+    # wait for first block generated before start testing
+    wait_for_block(cli, 1)
+
+    yield cli
+
+    duration = time.time() - begin
+    # wait for server startup complete to generate the coverage report
+    if duration < 15:
+        time.sleep(15 - duration)
+
+    supervisord.terminate()
+    supervisord.wait()
+
+    # collect the coverage results
+    txt = (data / "coverage.out").read_text()
+    merged = Path("coverage.txt")
+    if merged.exists():
+        assert txt.startswith("mode: set")
+        txt = txt[10:]
+    with merged.open("a") as f:
+        f.write(txt)

pystarport/default.nix

@@ -4,6 +4,6 @@ in
   pkgs.poetry2nix.mkPoetryApplication {
     projectDir = ./.;
     preBuild = ''
-    sed -i -e "s@CHAIN = 'chain-maind'  # edit by nix-build@CHAIN = '${chain}/bin/chain-maind'@" pystarport/cluster.py
+    sed -i -e 's@CHAIN = "chain-maind"  # edit by nix-build@CHAIN = "${chain}/bin/chain-maind"@' pystarport/cluster.py
     '';
   }

pystarport/pystarport/cluster.py

@@ -337,7 +337,7 @@ def init_cluster(data_dir, config, base_port, cmd=None):
     }
     for i, node in enumerate(config["validators"]):
         supervisord_ini[f"program:node{i}"] = {
-            "command": f"{cmd} start --home {home_dir(data_dir, i)}",
+            "command": f"{cmd} start --home %(here)s/node{i}",
             # redirect to supervisord's stdout, easier to collect all logs
             "stdout_logfile": "/dev/fd/1",
             "stdout_logfile_maxbytes": "0",

pytest.ini

@@ -0,0 +1,2 @@
+[pytest]
+testpaths = integration_tests

shell.nix

@@ -1,25 +1,18 @@
 { pkgs ? import <nixpkgs> {} }:
 with pkgs;
-let
-  chain = import ./. { inherit pkgs; };
-  pystarport = poetry2nix.mkPoetryEnv {
-    projectDir = ./pystarport;
-    editablePackageSources = {
-      pystarport = ./pystarport;
-    };
-  };
-in
-  mkShell {
-    buildInputs = [
-      go
-      chain
-      chain.instrumented
-      pystarport
-      python3Packages.poetry
-      python3Packages.pytest_xdist
-      python3Packages.pytest
-      python3Packages.flake8
-      python3Packages.black
-      python3Packages.isort
-    ];
-  }
+mkShell {
+  inputsFrom = [
+    # base env
+    (import ./integration_tests/shell.nix { inherit pkgs; })
+  ];
+  buildInputs = [
+    go
+    # make default chain-maind available on PATH
+    (import ./. { inherit pkgs; })
+    python3Packages.poetry
+  ];
+  shellHook = ''
+  # prefer local pystarport directory for development
+  export PYTHONPATH=./pystarport:$PYTHONPATH
+  '';
+}