Refactor the current codebase

.github/workflows/ci.yml

@@ -14,3 +14,29 @@ jobs:
         args: -v -r *.md
     - name: Fail if there were link errors
       run: exit ${{ steps.lc.outputs.exit_code }}
+
+  build:
+    name: CI tests
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - name: Set up Python 3.7
+      uses: actions/setup-python@v1
+      with:
+        python-version: 3.7
+    - name: Install dependencies
+      run: |
+        python -m pip install --upgrade pip
+        pip install -r requirements.txt
+    - name: Check formatting with blake
+      run: |
+        black --check . --exclude generator/generators/protobuf
+    - name: Lint with flake8
+      run: |
+        # The GitHub editor is 127 chars wide
+        flake8 . --count --max-line-length=127 --statistics --exclude generator/generators/protobuf
+    - name: Execute smoke test
+      run: |
+        python parsec_mock/parsec_mock.py &
+        sleep 5
+        python tests/mock_test.py

.gitmodules

@@ -1,3 +1,3 @@
 [submodule "parsec-operations"]
-	path = parsec-operations
+	path = generator/generators/parsec-operations
 	url = https://github.com/parallaxsecond/parsec-operations

generator/generator.py

@@ -9,7 +9,16 @@
 from struct import pack
 import base64
 
-from generator_lib import generators
+# Generator functions
+# Add your new generator function here
+# TODO: with some import syntax or clever moduling, it might be possible not
+# to have to do anything here?
+# F401 ignored for flake8 as those methods are called through eval
+from generators.ping_noauth import gen as ping_noauth  # noqa: F401
+from generators.list_opcodes_bad1 import gen as list_opcodes_bad1  # noqa: F401
+from generators.list_opcodes_directauth import (  # noqa: F401
+    gen as list_opcodes_directauth,
+)
 
 
 class TestSpec(object):
@@ -27,71 +36,68 @@ def _traverse(key, element):
         self.__dict__.update(objd)
         self.basedict = dictionary
 
-    def is_valid(self):
-        return True
-
 
 def read_specs(folder):
     """Read test specs from a folder"""
     specfiles = [f for f in listdir(folder) if isfile(join(folder, f))]
     specs = []
     for file in specfiles:
         print(f"Parsing spec file: {file}")
-        with open(os.path.join(folder, file), 'r') as f:
+        # Only use the first part of the filename as spec name
+        name = file.split(".")[0]
+        with open(os.path.join(folder, file), "r") as f:
             spec = safe_load(f)
             testspec = TestSpec(spec["spec"])
-            if testspec.is_valid():
-                specs.append(testspec)
-            else:
-                print(f"Error loading test spec from {file}")
+            specs.append((name, testspec))
     return specs
 
 
 def generate_data(specs, output_folder):
     """Generate test data for a list of specs"""
-    for spec in specs:
-        if spec.generator in generators:
-            print(f"Generating test {spec.name}")
-            generate_spec_data(output_folder, spec, generators[spec.generator])
-        else:
-            print(f"No generator function found for spec {spec.name}, skipping...")
+    for (name, spec) in specs:
+        generate_spec_data(output_folder, spec, name)
 
 
-def generate_spec_data(output_folder, spec, generator_fn):
+def generate_spec_data(output_folder, spec, name):
     """Generates data for a single spec and outputs it into the specified output folder."""
-    (operation, result) = generator_fn()
+    # The generator function has the same name as the test specification
+    (operation, result) = eval(name)()
 
     request_auth = create_auth(spec.request.auth)
     request_content_len = spec.request.header.content_length
-    if request_content_len == 'auto':
+    if request_content_len == "auto":
         request_content_len = len(operation)
 
     request_auth_len = spec.request.header.auth_length
-    if request_auth_len == 'auto':
+    if request_auth_len == "auto":
         request_auth_len = len(request_auth)
-    request_header = pack_header(spec.request.header, request_auth_len, request_content_len)
+    request_header = pack_header(
+        spec.request.header, request_auth_len, request_content_len
+    )
 
     response_content_len = spec.response.header.content_length
-    if response_content_len == 'auto':
+    if response_content_len == "auto":
         response_content_len = len(result)
 
     response_auth_len = spec.response.header.auth_length
-
-    response_header = pack_header(spec.response.header, response_auth_len, response_content_len)
+    response_header = pack_header(
+        spec.response.header, response_auth_len, response_content_len
+    )
 
     request_buf = request_header + operation + request_auth
     response_buf = response_header + result
 
+    # The generator appends the base64 data at the end of the spec file
     out_data = {
         "spec": spec.basedict,
         "test_data": {
-            "request": base64.b64encode(request_buf).decode('ascii'),
-            "response": base64.b64encode(response_buf).decode('ascii'),
-        }
+            "request": base64.b64encode(request_buf).decode("ascii"),
+            "response": base64.b64encode(response_buf).decode("ascii"),
+        },
     }
-    out_path = os.path.join(output_folder, spec.name + ".test.yaml")
-    print(f"Writing spec {spec.name} test data to {out_path}")
-    with open(out_path, 'w') as f:
+    out_path = os.path.join(output_folder, name + ".test.yaml")
+    print(f"Writing spec {name} test data to {out_path}")
+    with open(out_path, "w") as f:
         dump(out_data, f, sort_keys=False)
 
 
@@ -100,42 +106,48 @@ def pack_header(header, auth_len, body_len):
     # pack function converts arguments into binary string, based on format string.
     # < means integers are little endian.  Rest of format string is one character per input to indicate
     # packed field interpretation.  See struct.pack docs for details.
-    return pack('<IHBBHBQBBBIHIHH',
-                header.magic_number,
-                header.header_size,
-                header.major_version_number,
-                header.minor_version_number,
-                header.flags,
-                header.provider,
-                header.session_handle,
-                header.content_type,
-                header.accept_type,
-                header.auth_type,
-                body_len,
-                auth_len,
-                header.opcode,
-                header.status,
-                0
-                )
+    # This should map to the Fixed Common Header defined here:
+    # https://parallaxsecond.github.io/parsec-book/parsec_client/wire_protocol.html#the-fixed-common-header
+    return pack(
+        "<IHBBHBQBBBIHIHH",
+        header.magic_number,
+        header.header_size,
+        header.major_version_number,
+        header.minor_version_number,
+        header.flags,
+        header.provider,
+        header.session_handle,
+        header.content_type,
+        header.accept_type,
+        header.auth_type,
+        body_len,
+        auth_len,
+        header.opcode,
+        header.status,
+        0,
+    )
 
 
 def create_auth(auth_spec):
     """Creates auth body of message"""
-    if auth_spec.type == 'none':
-        return b''
-    if auth_spec.type == 'direct':
-        return auth_spec.app_name.encode('utf-8')
-    return b''
+    if auth_spec.type == "none":
+        return b""
+    if auth_spec.type == "direct":
+        return auth_spec.app_name.encode("utf-8")
+    return b""
 
 
 def main():
-    specdir = os.path.abspath(os.path.join(os.path.dirname(__file__), 'testspecs'))
-    datadir = os.path.abspath(os.path.join(os.path.dirname(__file__), '../testdata'))
     print("Generating test data.")
+
+    specdir = os.path.abspath(os.path.join(os.path.dirname(__file__), "test_specs"))
     print(f"Reading test specs from {specdir}")
-    print(f"Generating test data to {datadir}")
     specs = read_specs(specdir)
 
+    datadir = os.path.abspath(
+        os.path.join(os.path.dirname(__file__), "../generator_output")
+    )
+    print(f"Generating test data to {datadir}")
     generate_data(specs, datadir)
 
 

generator/generators/Makefile

@@ -0,0 +1,11 @@
+# Copyright 2021 Contributors to the Parsec project.
+# SPDX-License-Identifier: Apache-2.0
+PROTOC_OUTPUT_FILES=$(shell find parsec-operations/protobuf/ -name "*.proto" -exec basename {} .proto \; | awk '{print "protobuf/"$$1"_pb2.py"}')
+.PHONY: protobuf all
+
+all: protobuf
+
+protobuf: ${PROTOC_OUTPUT_FILES}
+
+protobuf/%_pb2.py: parsec-operations/protobuf/%.proto
+	@protoc -I=parsec-operations/protobuf --python_out=protobuf $< > /dev/null

generator/gen_list_operations.py → generator/generators/list_opcodes_bad1.py

@@ -1,13 +1,12 @@
 # Copyright 2021 Contributors to the Parsec project.
 # SPDX-License-Identifier: Apache-2.0
-import protobuf.ping_pb2
-import protobuf.list_opcodes_pb2
+from .protobuf import list_opcodes_pb2
 
 
-def gen_list_opcodes_auth_direct():
-    operation = protobuf.list_opcodes_pb2.Operation()
+def gen():
+    operation = list_opcodes_pb2.Operation()
     operation.provider_id = 1
 
-    result = protobuf.list_opcodes_pb2.Result()
+    result = list_opcodes_pb2.Result()
     result.opcodes.extend([1, 3, 2])
     return (operation.SerializeToString(), result.SerializeToString())

generator/generators/list_opcodes_directauth.py

@@ -0,0 +1,12 @@
+# Copyright 2021 Contributors to the Parsec project.
+# SPDX-License-Identifier: Apache-2.0
+from .protobuf import list_opcodes_pb2
+
+
+def gen():
+    operation = list_opcodes_pb2.Operation()
+    operation.provider_id = 1
+
+    result = list_opcodes_pb2.Result()
+    result.opcodes.extend([1, 3, 2, 6])
+    return (operation.SerializeToString(), result.SerializeToString())

generator/gen_ping_operations.py → generator/generators/ping_noauth.py

@@ -1,12 +1,11 @@
 # Copyright 2021 Contributors to the Parsec project.
 # SPDX-License-Identifier: Apache-2.0
-import protobuf.ping_pb2
-import protobuf.list_opcodes_pb2
+from .protobuf import ping_pb2
 
 
-def gen_ping_no_auth():
-    op = protobuf.ping_pb2.Operation()
-    result = protobuf.ping_pb2.Result()
+def gen():
+    op = ping_pb2.Operation()
+    result = ping_pb2.Result()
     result.wire_protocol_version_maj = 1
     result.wire_protocol_version_min = 0