Refresh protocol tests and skip known failures by pulimsr · Pull Request #3920 · aws/aws-sdk-cpp

pulimsr · 2026-09-04T20:41:33Z

Refreshes the C2J protocol tests and adds an explicit skip list so known failures don't mask results for the rest of the suite.

Test harness (`AwsProtocolTestHelpers.h`)

Output-test assertions run again. Since Protocol tests updates #3900, the 333 output tests that use ValidateRequestSent(callback) never ran their result assertions: that overload passes an empty ExpectedRequest, and the !expected.body.empty() guard skipped the callback. A new alwaysCompareBody flag (set only by that overload) restores them. Input tests are unchanged.
Header values are compared after normalizing whitespace around commas (RFC 7230 OWS), so a, b and a,b match. Note this applies to every header, also normalizes inside quoted values, and drops empty list entries (a,,b == a,b).

Runner (`run_protocol_tests.py`)

KNOWN_FAILURES: 193 annotated entries across 12 suites, passed to gtest as a negative filter. All entries match existing test names.

Test inputs / codegen

Refreshed C2J test models and cases, including new query-compatible and non-query-compatible rpcv2 CBOR suites.
Event-stream codegen (JsonEventStreamHandlerSource.vm, CppViewHelper, Shape, transformer): handlers now support header-bearing events, explicit
@eventPayload string/structure payloads, and byte/short/integer/long/blob/timestamp headers. Regenerating all 13 real JSON event-stream services is byte-identical; only the rest-json test client gains its 4 output/duplex stream operations.
protocol_tests_gen.py filters only input-only event-stream operations (unsupported by C2J; no AWS service models one).
Generated tests that use NaN/Infinity include <cmath>/<limits> (GCC).
tinyxml2 is a system include in the protocol-test CMake (-Werror=type-limits).

Check all that applies:

Did a review by yourself.
Added proper tests to cover this PR. (If tests are not applicable, explain.)
Checked if this PR is a breaking (APIs have been changed) change.
Checked if this PR will not introduce cross-platform inconsistent behavior.
Checked if this PR would require a ReadMe/Wiki update.

Check which platforms you have built SDK on to verify the correctness of this PR.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

sbiscigl · 2026-10-19T18:03:15Z

+    const Aws::Vector<Aws::String> parts = Aws::Utils::StringUtils::Split(value, ',');
+    Aws::String joined;
+    for (const auto& part : parts) {
+      if (!joined.empty()) {


can you give me a example where this would be useful? what header values would cause this to happen? the method seems like all we do is split up a thing in a list of strings then re-construct it. additionally likely should use std::accumulate to accomplish this task. we do it loads of other places in the code base

It fixes SDKAppendedGzipAfterProvidedEncoding (restJson1/restXml): the test expects custom, gzip and we send custom,gzip. Both are valid since whitespace around commas in a list header isn't significant (RFC 9110 5.6.1), so the helper normalizes it before comparing. Also switched it to std::accumulate

sbiscigl · 2026-10-19T18:04:59Z

    }
    // A Smithy vector with no "body" asserts only headers/uri, not the body.
-    if (!expected.body.empty()) {
+    if (alwaysCompareBody || !expected.body.empty()) {


whats the use case for adding a boolean alwaysCompareBody when bodyCompare could be no-op passed lambda?

Moved the empty-body check into the body comparators, so they're no-ops when no body is expected, and dropped the flag. Output tests' callbacks now always run.

sbiscigl · 2026-10-19T18:07:03Z

@@ -22,7 +22,7 @@ mkdir "${PREFIX_DIR}/al2-install"
 cd "${PREFIX_DIR}/al2-build"
 CMAKE_ARGS=(-GNinja ../aws-sdk-cpp -DCMAKE_BUILD_TYPE="$BUILD_TYPE" -DMINIMIZE_SIZE=ON -DCMAKE_INSTALL_PREFIX="${PREFIX_DIR}/al2-install" -DBUILD_PERFORMANCE_TESTS=ON -DAWS_ENABLE_CORE_INTEGRATION_TEST=ON -DENABLE_SMOKE_TESTS=ON)
 if [ "$BUILD_TYPE" = "Debug" ]; then
-  CMAKE_ARGS+=(-DCMAKE_CXX_FLAGS="-ggdb -fsanitize=address")
+  CMAKE_ARGS+=(-DCMAKE_CXX_FLAGS="-ggdb -fsanitize=address" -DENABLE_PROTOCOL_TESTS=ON)


lets talk about this offline

sbiscigl · 2026-10-19T18:08:32Z

+done
+export LD_LIBRARY_PATH
+# Mock server dependency.
+python3 -c 'import flask' 2>/dev/null || python3 -m pip install --user flask


pulling in a python dependency on flask, when we run tests shouldnt be something that happens at test time, it should be a property of the CI running it.

removed the script

sbiscigl · 2026-10-19T18:09:59Z

@@ -18,6 +18,239 @@
 SCRIPTS_DIR = os.path.dirname(__file__)
 PROTO_TEST_MOCK_HANDLER = f"{SCRIPTS_DIR}/protocol_tests_mock_server/protocol_tests_mock_server.py"

+# Tests that do not pass with the legacy C2J-generated clients (fail, crash or hang), per (protocol, group).


do any of these tests exhibit anything in common, would you expect any new tests to fail when addded?

Yes, most of them fall into a few groups: limitations of the mock server (response headers, percent-decoded paths/queries, and the host forced to localhost), plus some real SDK gaps.

And yes, new tests could fail when added, but since we add them manually, we'd triage any failures at that point and update the skip list.

pulimsr force-pushed the protocol-tests-skip-known-failures branch 3 times, most recently from 616c33c to 1b375c4 Compare October 13, 2026 05:49

pulimsr added 5 commits October 14, 2026 10:52

Compare protocol-test list headers semantically; add runner skip list

cf73f92

updating the protocol tests and skipping known failures

0b9964b

regenerating the protocol tests

b8783a1

adding protocol tests to the AL2 debug build

441ee2d

adding cmath and limits includes to protocol tests

b4d9421

pulimsr force-pushed the protocol-tests-skip-known-failures branch from 1b375c4 to b4d9421 Compare October 14, 2026 15:08

pulimsr added 2 commits October 14, 2026 14:21

adding event stream codegen support for protocol tests

93f52d5

regenerating the protocol tests

c3a1852

pulimsr changed the title ~~Compare protocol-test list headers semantically; add runner skip list~~ Refresh protocol tests and skip known failures Oct 19, 2026

updating the protocol tests changelog

c0664ad

sbiscigl reviewed Oct 19, 2026

View reviewed changes

pulimsr requested a review from kai-ion October 19, 2026 18:16

removing the protocol test CI script

dd6907d

pulimsr force-pushed the protocol-tests-skip-known-failures branch from 66af1f9 to dd6907d Compare October 19, 2026 20:57

pulimsr added 2 commits October 21, 2026 15:14

using std::accumulate in CanonicalizeHeaderValue and fixing formatting

ea7cc19

moving the empty-body check into the body comparators

80e5afe

pulimsr requested review from sbiscigl and removed request for sbiscigl October 21, 2026 19:58

kai-ion approved these changes Oct 22, 2026

View reviewed changes

Merge branch 'main' into protocol-tests-skip-known-failures

1e14a9d

Navigation Menu

Repository navigation

Refresh protocol tests and skip known failures - #3920

Refresh protocol tests and skip known failures #3920
pulimsr wants to merge 12 commits into
mainfrom
protocol-tests-skip-known-failures

pulimsr commented Sep 4, 2026 •

edited

Loading

Uh oh!

sbiscigl Oct 19, 2026

Uh oh!

pulimsr Oct 21, 2026

Uh oh!

sbiscigl Oct 19, 2026

Uh oh!

pulimsr Oct 21, 2026

Uh oh!

sbiscigl Oct 19, 2026

Uh oh!

sbiscigl Oct 19, 2026

Uh oh!

pulimsr Oct 21, 2026

Uh oh!

sbiscigl Oct 19, 2026

Uh oh!

pulimsr Oct 19, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

pulimsr commented Sep 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test harness (AwsProtocolTestHelpers.h)

Runner (run_protocol_tests.py)

Test inputs / codegen

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pulimsr commented Sep 4, 2026 •

edited

Loading

Test harness (`AwsProtocolTestHelpers.h`)

Runner (`run_protocol_tests.py`)