Skip to content

Commit 650d72f

Browse files
committed
fix review comments
Signed-off-by: john 9e9 <[email protected]>
1 parent dc87727 commit 650d72f

12 files changed

+115
-112
lines changed

.gitignore

+1
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
/target
22
*patch
33
.devcontainer
4+
.vscode

README.md

+9-11
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ To write a new test, a developer should create a test spec file in the generator
5858
5959
They then need to write a generator function in ```generator/generator_lib.py```. This generator function takes no arguments, and should output a tuple of python binary strings.
6060

61-
The first element of the tuple should be the protocol buffer encoded value of the request message that is described in the spec.request.body_description field of the test spec. The second element of the tuple should be the protocol buffer encoded value of the result message that is described in the spec.result.body_description field of the test spec.
61+
The first element of the tuple should be the protocol buffer encoded value of the request's *operation* message that is described in the spec.request.body_description field of the test spec. The second element of the tuple should be the protocol buffer encoded *result* value of the response message that is described in the spec.result.body_description field of the test spec.
6262

6363
An example generator for a list opcodes test is shown below:
6464

@@ -134,14 +134,13 @@ spec:
134134
auth:
135135
type: direct
136136
app_name: jimbob
137-
expected_parse_result: fail_auth
138137
139138
```
140139
The whole test spec is defined in the spec: object in the file. In that spec, the fields are:
141140
| Field | Description |
142141
| --- | --- |
143142
| name | The name of the test spec. Used to name the output test data file |
144-
| generator | The lookup for the request and response generator in the generator library. See [writing tests](#writing-tests).|
143+
| generator | The lookup for the operation and result generator in the generator library. See [writing tests](#writing-tests).|
145144
| description | Description of test, not used in generation |
146145
| request | Settings for configuring the request data for the test data file |
147146
| request.header | Field values for the request header. Meanings of these files and valid values can be found in the [parsec book](https://parallaxsecond.github.io/parsec-book/parsec_client/wire_protocol.html). |
@@ -150,14 +149,13 @@ The whole test spec is defined in the spec: object in the file. In that spec, t
150149
| request.body_description | A free form description of the contents of the request content. Used for test writers (and generator writers) to understand how to construct the request object. This field is not used by the test generator. |
151150
| request.auth.type | This can either be ```none```, which will cause the auth section of the message to be empty (equivalent of the No Authentication type of authentication); or it can be ```direct```, which will cause the auth section of the message to contain authentication data corresponding to the format for Direct Authentication. If ```direct``` is set, then the request.auth.app_name field must be set. Note that this field does not cause the header auth_type field to be set. |
152151
| request.auth.app_name | Used to populate the auth section of the message when ```direct``` authentication is selected |
153-
| result | Settings for configuration the result data for the test data file |
154-
| result.header | Field values for the result header. Meanings of these files and valid values can be found in the [parsec book](https://parallaxsecond.github.io/parsec-book/parsec_client/wire_protocol.html). |
155-
| result.header.content_length | If this field has the value ```auto``` then its value is calculated from the generated result content. If the value is a number, then that value is used in the field instead |
156-
| result.header.auth_length | If this field has the value ```auto``` then its value is calculated from the generated auth content. If the value is a number, then that value is used in the field instead. |
157-
| result.body_description | A free form description of the contents of the result content. Used for test writers (and generator writers) to understand how to construct the request object. This field is not used by the test generator. |
158-
| result.auth.type | This can either be ```none```, which will cause the auth section of the message to be empty (equivalent of the No Authentication type of authentication); or it can be ```direct```, which will cause the auth section of the message to contain authentication data corresponding to the format for Direct Authentication. If ```direct``` is set, then the result.auth.app_name field must be set. Note that this field does not cause the header auth_type field to be set. **NOTE:** A Parsec client would not send authentication data in a result message, but this spec format allows test authors to create the message as they wish to excersice the parsec client code. |
159-
| result.auth.app_name | Used to populate the auth section of the message when ```direct``` authentication is selected |
160-
| result.expected_parse_result | Used to indicate whether a client being tested with this data should successfully be parsed (even if it is returning a failure status code). If the message is valid according to the parsec interface specification, this field should have the valid ```succeed```. If the message is invalid, it should have the values of ```fail_header``` if the header is invalid; ```fail_auth``` if the authentication data is invlid, or ```fail_content``` if the content is invalid. Further values may be added to this ennum in the future. |
152+
| response | Settings for configuration the response data for the test data file |
153+
| response.header | Field values for the response header. Meanings of these files and valid values can be found in the [parsec book](https://parallaxsecond.github.io/parsec-book/parsec_client/wire_protocol.html). |
154+
| response.header.content_length | If this field has the value ```auto``` then its value is calculated from the generated response content. If the value is a number, then that value is used in the field instead |
155+
| response.header.auth_length | If this field has the value ```auto``` then its value is calculated from the generated auth content. If the value is a number, then that value is used in the field instead. |
156+
| response.body_description | A free form description of the contents of the response content. Used for test writers (and generator writers) to understand how to construct the result object. This field is not used by the test generator. |
157+
| response.auth.type | This can either be ```none```, which will cause the auth section of the message to be empty (equivalent of the No Authentication type of authentication); or it can be ```direct```, which will cause the auth section of the message to contain authentication data corresponding to the format for Direct Authentication. If ```direct``` is set, then the result.auth.app_name field must be set. Note that this field does not cause the header auth_type field to be set. **NOTE:** A Parsec client would not send authentication data in a result message, but this spec format allows test authors to create the message as they wish to excersice the parsec client code. |
158+
| response.auth.app_name | Used to populate the auth section of the message when ```direct``` authentication is selected |
161159

162160

163161

generator/gen_list_operations.py

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
# Copyright 2021 Contributors to the Parsec project.
2+
# SPDX-License-Identifier: Apache-2.0
3+
import protobuf.ping_pb2
4+
import protobuf.list_opcodes_pb2
5+
6+
7+
def gen_list_opcodes_auth_direct():
8+
operation = protobuf.list_opcodes_pb2.Operation()
9+
operation.provider_id = 1
10+
11+
result = protobuf.list_opcodes_pb2.Result()
12+
result.opcodes.extend([1, 3, 2])
13+
return (operation.SerializeToString(), result.SerializeToString())

generator/gen_ping_operations.py

+13
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
# Copyright 2021 Contributors to the Parsec project.
2+
# SPDX-License-Identifier: Apache-2.0
3+
import protobuf.ping_pb2
4+
import protobuf.list_opcodes_pb2
5+
6+
7+
def gen_ping_no_auth():
8+
op = protobuf.ping_pb2.Operation()
9+
result = protobuf.ping_pb2.Result()
10+
result.wire_protocol_version_maj = 1
11+
result.wire_protocol_version_min = 0
12+
13+
return (op.SerializeToString(), result.SerializeToString())

generator/generator.py

+60-61
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,17 @@
44
from os import listdir
55
from os.path import isfile, join
66

7-
from yaml import load, safe_load, dump
8-
try:
9-
from yaml import CLoader as Loader, CDumper as Dumper
10-
except ImportError:
11-
from yaml import Loader, Dumper
7+
from yaml import safe_load, dump
128

139
from struct import pack
1410
import base64
1511

1612
from generator_lib import generators
1713

18-
# Class to represent a test specification. Used to convert
19-
# dictionary created by pyaml to object format, making code easier to read.
14+
2015
class TestSpec(object):
16+
"""Class to represent a test specification. Used to convert
17+
dictionary created by pyaml to object format, making code easier to read."""
2118

2219
def __init__(self, dictionary):
2320
def _traverse(key, element):
@@ -29,18 +26,18 @@ def _traverse(key, element):
2926
objd = dict(_traverse(k, v) for k, v in dictionary.items())
3027
self.__dict__.update(objd)
3128
self.basedict = dictionary
32-
29+
3330
def is_valid(self):
3431
return True
3532

3633

37-
# Read test specs from a folder
3834
def read_specs(folder):
35+
"""Read test specs from a folder"""
3936
specfiles = [f for f in listdir(folder) if isfile(join(folder, f))]
4037
specs = []
4138
for file in specfiles:
4239
print(f"Parsing spec file: {file}")
43-
with open(os.path.join(folder,file), 'r') as f:
40+
with open(os.path.join(folder, file), 'r') as f:
4441
spec = safe_load(f)
4542
testspec = TestSpec(spec["spec"])
4643
if testspec.is_valid():
@@ -49,96 +46,98 @@ def read_specs(folder):
4946
print(f"Error loading test spec from {file}")
5047
return specs
5148

52-
# Generate test data for a list of specs
49+
5350
def generate_data(specs, output_folder):
51+
"""Generate test data for a list of specs"""
5452
for spec in specs:
5553
if spec.generator in generators:
5654
print(f"Generating test {spec.name}")
5755
generate_spec_data(output_folder, spec, generators[spec.generator])
5856
else:
5957
print(f"No generator function found for spec {spec.name}, skipping...")
6058

61-
# Generates data for a single spec and outputs it into the specified output folder
62-
def generate_spec_data(output_folder,spec, generator_fn):
63-
(op, result) = generator_fn()
64-
65-
op_auth = create_auth(spec.request.auth)
66-
op_content_len = spec.request.header.content_length
67-
if op_content_len == 'auto':
68-
op_content_len = len(op)
69-
70-
op_auth_len = spec.request.header.auth_length
71-
if op_auth_len == 'auto':
72-
op_auth_len = len(op_auth)
73-
op_header = pack_header(spec.request.header, op_auth_len, op_content_len)
74-
75-
76-
result_content_len = spec.result.header.content_length
77-
if result_content_len == 'auto':
78-
result_content_len = len(result)
79-
80-
result_auth = create_auth(spec.result.auth)
81-
result_auth_len = spec.result.header.auth_length
82-
if result_auth_len == 'auto':
83-
result_auth_len = len(result_auth)
84-
85-
result_header = pack_header(spec.result.header, result_auth_len, result_content_len)
86-
87-
op_buf = op_header+op_auth+op
88-
result_buf = result_header + result_auth + result
59+
60+
def generate_spec_data(output_folder, spec, generator_fn):
61+
"""Generates data for a single spec and outputs it into the specified output folder."""
62+
(operation, result) = generator_fn()
63+
64+
request_auth = create_auth(spec.request.auth)
65+
request_content_len = spec.request.header.content_length
66+
if request_content_len == 'auto':
67+
request_content_len = len(operation)
68+
69+
request_auth_len = spec.request.header.auth_length
70+
if request_auth_len == 'auto':
71+
request_auth_len = len(request_auth)
72+
request_header = pack_header(spec.request.header, request_auth_len, request_content_len)
73+
74+
response_content_len = spec.response.header.content_length
75+
if response_content_len == 'auto':
76+
response_content_len = len(result)
77+
78+
response_auth_len = spec.response.header.auth_length
79+
80+
response_header = pack_header(spec.response.header, response_auth_len, response_content_len)
81+
82+
request_buf = request_header + operation + request_auth
83+
response_buf = response_header + result
8984

9085
out_data = {
9186
"spec": spec.basedict,
9287
"test_data": {
93-
"request": base64.b64encode(op_buf).decode('ascii'),
94-
"result": base64.b64encode(result_buf).decode('ascii'),
88+
"request": base64.b64encode(request_buf).decode('ascii'),
89+
"response": base64.b64encode(response_buf).decode('ascii'),
9590
}
9691
}
9792
out_path = os.path.join(output_folder, spec.name + ".test.yaml")
9893
print(f"Writing spec {spec.name} test data to {out_path}")
9994
with open(out_path, 'w') as f:
10095
dump(out_data, f, sort_keys=False)
10196

102-
# Take a header data structure and convert it into binary representation.
97+
10398
def pack_header(header, auth_len, body_len):
99+
"""Take a header data structure and convert it into binary representation."""
104100
# pack function converts arguments into binary string, based on format string.
105101
# < means integers are little endian. Rest of format string is one character per input to indicate
106102
# packed field interpretation. See struct.pack docs for details.
107103
return pack('<IHBBHBQBBBIHIHH',
108-
header.magic_number,
109-
header.header_size,
110-
header.major_version_number,
111-
header.minor_version_number,
112-
header.flags,
113-
header.provider,
114-
header.session_handle,
115-
header.content_type,
116-
header.accept_type,
117-
header.auth_type,
118-
body_len,
119-
auth_len,
120-
header.opcode,
121-
header.status,
122-
0
123-
)
124-
125-
# Creates auth body of message
104+
header.magic_number,
105+
header.header_size,
106+
header.major_version_number,
107+
header.minor_version_number,
108+
header.flags,
109+
header.provider,
110+
header.session_handle,
111+
header.content_type,
112+
header.accept_type,
113+
header.auth_type,
114+
body_len,
115+
auth_len,
116+
header.opcode,
117+
header.status,
118+
0
119+
)
120+
121+
126122
def create_auth(auth_spec):
123+
"""Creates auth body of message"""
127124
if auth_spec.type == 'none':
128125
return b''
129126
if auth_spec.type == 'direct':
130127
return auth_spec.app_name.encode('utf-8')
131128
return b''
132129

130+
133131
def main():
134132
specdir = os.path.abspath(os.path.join(os.path.dirname(__file__), 'testspecs'))
135133
datadir = os.path.abspath(os.path.join(os.path.dirname(__file__), '../testdata'))
136134
print("Generating test data.")
137135
print(f"Reading test specs from {specdir}")
138136
print(f"Generating test data to {datadir}")
139137
specs = read_specs(specdir)
140-
138+
141139
generate_data(specs, datadir)
142140

141+
143142
if __name__ == "__main__":
144143
main()

generator/generator_lib.py

+4-19
Original file line numberDiff line numberDiff line change
@@ -1,26 +1,11 @@
11
# Copyright 2021 Contributors to the Parsec project.
22
# SPDX-License-Identifier: Apache-2.0
3-
import protobuf.ping_pb2
4-
import protobuf.list_opcodes_pb2
3+
from gen_list_operations import gen_list_opcodes_auth_direct
4+
from gen_ping_operations import gen_ping_no_auth
55

66

7-
def gen_ping_no_auth():
8-
op = protobuf.ping_pb2.Operation()
9-
result = protobuf.ping_pb2.Result()
10-
result.wire_protocol_version_maj = 1
11-
result.wire_protocol_version_min = 0
12-
13-
14-
return (op.SerializeToString(), result.SerializeToString())
15-
16-
def gen_list_opcodes_auth_direct():
17-
op = protobuf.list_opcodes_pb2.Operation()
18-
op.provider_id = 1
19-
20-
result = protobuf.list_opcodes_pb2.Result()
21-
result.opcodes.extend([1,3,2])
22-
return (op.SerializeToString(), result.SerializeToString())
23-
7+
# Dictionary for generators. The values in this dictionary must be functions to produce a tuple
8+
# (operation, result) of binary strings.
249
generators = {
2510
'ping_no_auth': gen_ping_no_auth,
2611
'list_opcodes_auth_direct': gen_list_opcodes_auth_direct,

generator/testspecs/list_opcodes_bad1.spec.yaml

+2-3
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ spec:
2424
auth:
2525
type: direct
2626
app_name: jimbob
27-
result:
27+
response:
2828
header:
2929
magic_number: 0x5EC0A710
3030
header_size: 0x1E
@@ -37,11 +37,10 @@ spec:
3737
accept_type: 0x00
3838
auth_type: 0x00
3939
content_length: 500
40-
auth_length: auto
40+
auth_length: 0
4141
opcode: 0x00000009
4242
status: 0x0000
4343
body_description: list opcodes result [1,3,2]
4444
auth:
4545
type: direct
4646
app_name: jimbob
47-
expected_parse_result: succeed

generator/testspecs/list_opcodes_directauth.spec.yaml

+2-3
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ spec:
2424
auth:
2525
type: direct
2626
app_name: jimbob
27-
result:
27+
response:
2828
header:
2929
magic_number: 0x5EC0A710
3030
header_size: 0x1E
@@ -37,11 +37,10 @@ spec:
3737
accept_type: 0x00
3838
auth_type: 0x00
3939
content_length: auto
40-
auth_length: auto
40+
auth_length: 0
4141
opcode: 0x00000009
4242
status: 0x0000
4343
body_description: list opcodes result [1,3,2]
4444
auth:
4545
type: direct
4646
app_name: jimbob
47-
expected_parse_result: succeed

generator/testspecs/ping_noauth.spec.yaml

+1-2
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ spec:
2323
body_description: no parameters required
2424
auth:
2525
type: none
26-
result:
26+
response:
2727
header:
2828
magic_number: 0x5EC0A710
2929
header_size: 0x1E
@@ -42,4 +42,3 @@ spec:
4242
body_description: Ping response with major version 1 minor version 0
4343
auth:
4444
type: none
45-
expected_parse_result: succeed

testdata/list_opcodes_auth_direct.test.yaml

+4-5
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ spec:
2222
auth:
2323
type: direct
2424
app_name: jimbob
25-
result:
25+
response:
2626
header:
2727
magic_number: 1589683984
2828
header_size: 30
@@ -35,14 +35,13 @@ spec:
3535
accept_type: 0
3636
auth_type: 0
3737
content_length: auto
38-
auth_length: auto
38+
auth_length: 0
3939
opcode: 9
4040
status: 0
4141
body_description: list opcodes result [1,3,2]
4242
auth:
4343
type: direct
4444
app_name: jimbob
45-
expected_parse_result: succeed
4645
test_data:
47-
request: EKfAXh4AAQAAAAAAAAAAAAAAAAAAAAIAAAAGAAkAAAAAAAAAamltYm9iCAE=
48-
result: EKfAXh4AAQAAAAAAAAAAAAAAAAAAAAUAAAAGAAkAAAAAAAAAamltYm9iCgMBAwI=
46+
request: EKfAXh4AAQAAAAAAAAAAAAAAAAAAAAIAAAAGAAkAAAAAAAAACAFqaW1ib2I=
47+
response: EKfAXh4AAQAAAAAAAAAAAAAAAAAAAAUAAAAAAAkAAAAAAAAACgMBAwI=

0 commit comments

Comments
 (0)