Skip to content

Commit a9e5c01

Browse files
SergeyRyabininSergeyRyabinin
committed
Code review comments
1 parent 22e082d commit a9e5c01

File tree

4 files changed

+61
-76
lines changed

4 files changed

+61
-76
lines changed

‎tools/code-generation/generator/src/main/java/com/amazonaws/util/awsclientgenerator/generators/cpp/CppProtocolTestGenerator.java

Lines changed: 7 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@
3030
import java.util.*;
3131
import java.util.stream.Collectors;
3232

33+
import static com.amazonaws.util.awsclientgenerator.generators.cpp.ec2.Ec2CppClientGenerator.legacyPatchEc2Model;
34+
3335
public class CppProtocolTestGenerator implements ClientGenerator {
3436

3537
private static String CMAKE_LISTS_TEMPLATE = "/com/amazonaws/util/awsclientgenerator/velocity/cpp/protocoltests/ProtocolTestsCMakeLists.vm";
@@ -57,7 +59,7 @@ public CppProtocolTestGenerator(ServiceModel serviceModel, ProtocolTestModel tes
5759

5860
velocityEngine.init();
5961

60-
legacyPatchEc2Model();
62+
legacyPatchEc2Model(this.serviceModel);
6163
}
6264

6365
protected SdkFileEntry generateTestSuiteSourceFile(ProtocolTestSuite testSuite) throws IOException {
@@ -165,42 +167,14 @@ private List<String> computeTestSourceIncludes(ProtocolTestSuite testSuite) {
165167
.collect(Collectors.toSet());
166168
// include the request shapes
167169
Set<String> requestShapes = testSuite.getCases().stream()
168-
.map(entry -> {
169-
Set<String> includes = new HashSet<>();
170-
includes.add(String.format("<aws/%s/model/%s.h>", serviceModel.getMetadata().getProjectName(),
171-
entry.getGiven().getName() + "Request"));
172-
173-
return includes;
174-
})
175-
.flatMap(entrySet -> entrySet.stream())
170+
.map(entry ->
171+
String.format("<aws/%s/model/%s.h>",
172+
serviceModel.getMetadata().getProjectName(),
173+
entry.getGiven().getName() + "Request"))
176174
.collect(Collectors.toSet());
177175

178176
set.addAll(requestShapes);
179177
set.removeAll(HEADERS_TO_NOT_INCLUDE);
180178
return set.stream().sorted().collect(Collectors.toList());
181179
}
182-
183-
/**
184-
* Perform legacy patching of the ec2 model present from the very beginning.
185-
*/
186-
private void legacyPatchEc2Model() {
187-
if (!serviceModel.getMetadata().getProtocol().equals("ec2")) {
188-
return;
189-
}
190-
191-
List<String> keysToRename = new LinkedList<>();
192-
Map<String, Shape> shapes = serviceModel.getShapes();
193-
for (final String key : shapes.keySet()) {
194-
final Shape shape = shapes.get(key);
195-
shape.setName(shape.getName().replaceAll("Result$", "Response"));
196-
shape.setType(shape.getType().replaceAll("Result$", "Response"));
197-
keysToRename.add(key);
198-
}
199-
200-
for (final String key : keysToRename) {
201-
final Shape shape = shapes.get(key);
202-
shapes.remove(key);
203-
shapes.put(key.replaceAll("Result$", "Response"), shape);
204-
}
205-
}
206180
}

‎tools/code-generation/generator/src/main/java/com/amazonaws/util/awsclientgenerator/generators/cpp/ec2/Ec2CppClientGenerator.java

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,13 @@ public Ec2CppClientGenerator() throws Exception {
3232
super();
3333
}
3434

35-
@Override
36-
public SdkFileEntry[] generateSourceFiles(ServiceModel serviceModel) throws Exception {
35+
/**
36+
* Perform legacy patching of the ec2 model present from the very beginning.
37+
*/
38+
public static void legacyPatchEc2Model(ServiceModel serviceModel) {
39+
if (!serviceModel.getMetadata().getProtocol().equals("ec2")) {
40+
return;
41+
}
3742

3843
List<String> keysToRename = new LinkedList<>();
3944
Map<String, Shape> shapes = serviceModel.getShapes();
@@ -49,7 +54,12 @@ public SdkFileEntry[] generateSourceFiles(ServiceModel serviceModel) throws Exce
4954
shapes.remove(key);
5055
shapes.put(key.replaceAll("Result$", "Response"), shape);
5156
}
57+
}
5258

59+
@Override
60+
public SdkFileEntry[] generateSourceFiles(ServiceModel serviceModel) throws Exception {
61+
legacyPatchEc2Model(serviceModel);
62+
Map<String, Shape> shapes = serviceModel.getShapes();
5363

5464
if (shapes.containsKey("SpotInstanceState")) {
5565
// add "disabled" state to SpotInstanceState

‎tools/code-generation/generator/src/main/resources/com/amazonaws/util/awsclientgenerator/velocity/cpp/protocoltests/ProtocolTestsCMakeLists.vm

Lines changed: 1 addition & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,7 @@ if(MSVC AND BUILD_SHARED_LIBS)
1717
add_definitions(-DGTEST_LINKED_AS_SHARED_LIBRARY=1)
1818
endif()
1919

20-
if (CMAKE_CROSSCOMPILING)
21-
set(AUTORUN_UNIT_TESTS OFF)
22-
endif()
23-
24-
if (AUTORUN_UNIT_TESTS)
25-
enable_testing()
26-
endif()
20+
enable_testing()
2721

2822
if(PLATFORM_ANDROID AND BUILD_SHARED_LIBS)
2923
add_library(${PROJECT_NAME} "${awsProjectProtocolTestSrcVar}")
@@ -36,12 +30,6 @@ set_compiler_warnings(${PROJECT_NAME})
3630

3731
target_link_libraries(${PROJECT_NAME} ${PROJECT_LIBS})
3832

39-
if (AUTORUN_UNIT_TESTS)
40-
ADD_CUSTOM_COMMAND( TARGET ${PROJECT_NAME} POST_BUILD
41-
COMMAND ${CMAKE_COMMAND} -E env LD_LIBRARY_PATH=${AWS_AUTORUN_LD_LIBRARY_PATH}:$ENV{LD_LIBRARY_PATH} $<TARGET_FILE:${PROJECT_NAME}>
42-
ARGS "--gtest_brief=1")
43-
endif()
44-
4533
if(NOT CMAKE_CROSSCOMPILING)
4634
SET_TARGET_PROPERTIES(${PROJECT_NAME} PROPERTIES OUTPUT_NAME ${PROJECT_NAME})
4735
endif()

‎tools/scripts/run_protocol_tests.py

Lines changed: 41 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -39,63 +39,76 @@ class MockHttpServerHandle:
3939
"""
4040

4141
def __init__(self):
42-
python_cmd = [shutil.which("python3"), PROTO_TEST_MOCK_HANDLER]
43-
self.process = subprocess.Popen(python_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
44-
print(f"Started mock server with PID {self.process.pid}")
45-
46-
def __del__(self):
47-
"""Destructor that sends Ctrl-C/SIGINT to the subprocess."""
42+
try:
43+
python_cmd = [shutil.which("python3"), PROTO_TEST_MOCK_HANDLER]
44+
self.process = subprocess.Popen(python_cmd, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
45+
print(f"Started mock server with PID {self.process.pid}")
46+
except Exception as exc:
47+
print(f"ERROR: unable to start Mock server:\n{exc}")
48+
raise exc
49+
50+
def stop(self):
51+
"""Sends Ctrl-C/SIGINT to the subprocess."""
4852
if self.process and self.process.poll() is None:
4953
print(f"Sending SIGINT to subprocess with PID {self.process.pid}")
5054
if os.name == 'nt': # Windows
5155
self.process.send_signal(signal.CTRL_C_EVENT)
5256
else: # Unix-like
5357
self.process.send_signal(signal.SIGINT)
5458

55-
time.sleep(0.5)
59+
# wait for process to actually terminate
60+
time.sleep(1)
5661

5762
if self.process.poll() is None:
5863
print(f"Process didn't terminate, sending SIGTERM to PID {self.process.pid}")
5964
self.process.terminate()
6065

61-
self.process.wait()
66+
try:
67+
self.process.wait(timeout=60)
68+
except subprocess.TimeoutExpired as exc:
69+
print(f"Subprocess with PID {self.process.pid} did not terminate after 1 minute: {exc}")
70+
print(f"Mock subprocess logs:\n{self.process.communicate()[0]}\n")
71+
raise exc
72+
6273
print(f"Subprocess with PID {self.process.pid} terminated")
6374
print(f"Mock http logs:\n{self.process.communicate()[0]}\n")
75+
self.process = None
6476

65-
def is_running(self):
66-
"""Check if the subprocess is still running."""
67-
return self.process.poll() is None if self.process else False
77+
def __enter__(self):
78+
return self
6879

69-
def wait(self):
70-
"""Wait for the subprocess to complete."""
71-
if self.process:
72-
return self.process.wait()
73-
return None
80+
def __exit__(self, exc_type, exc_value, exc_traceback):
81+
self.stop()
82+
83+
def __del__(self):
84+
self.stop()
7485

7586

7687
class ProtocolTestRunner:
7788
"""Actual test runner: collects available tests and runs them
7889
"""
7990

8091
def __init__(self, args):
81-
self.debug = args.get("debug", False)
8292
self.build_dir = args.get("build_dir")
8393
self.args = args
8494
self.fail = False
8595

8696
def run(self):
87-
tests = self._collect_tests()
88-
if not tests or not len(tests):
89-
print(f"No protocol tests found in {self.build_dir}")
90-
self.fail = True
91-
for test in tests:
92-
mock_server = MockHttpServerHandle()
93-
try:
94-
process = subprocess.run([test], timeout=6 * 60, check=True)
95-
process.check_returncode()
96-
except Exception as exc:
97-
print(f"Protocol test {test} failed with exception: {exc}")
97+
try:
98+
tests = self._collect_tests()
99+
if not tests or not len(tests):
100+
print(f"No protocol tests found in {self.build_dir}")
98101
self.fail = True
102+
for test in tests:
103+
with MockHttpServerHandle() as mock_server:
104+
try:
105+
subprocess.run([test], timeout=6 * 60, check=True)
106+
except Exception as exc:
107+
print(f"Protocol test {test} failed with exception: {exc}")
108+
self.fail = True
109+
except Exception as exc:
110+
print(f"Protocol tests failed with exception: {exc}")
111+
self.fail = True
99112

100113
def _collect_tests(self):
101114
all_tests = []

0 commit comments

Comments
 (0)