APIGW: migrate TestInvokeMethod to NextGen #12514

bentsku · 2025-04-10T19:50:12Z

Motivation

The TestInvokeMethod operation was still relying on the legacy invocation logic of API Gateway. This PR migrates it to not rely on old code anymore and use the new handler chain, create one and invoke it.

I've also improved the test, it is now fully transformed and can run against AWS with snapshot validation enabled.

Also took the opportunity to remove some old "fixtures" doing nothing.

Changes

implement new TestInvokeOperation for the NextGen provider
create new file keeping all of this together (might be better organized to be honest... but it's also not very important at the moment? should it have better encapsulation?)
adapt the existing test to allow better parity
clean up some old APIGW "fixtures"

github-actions · 2025-04-10T20:18:38Z

LocalStack Community integration with Pro

2 files ± 0 2 suites ±0 29m 17s ⏱️ - 1h 23m 39s
1 102 tests - 3 248 1 037 ✅ - 2 946 65 💤 - 302 0 ❌ ±0
1 104 runs - 3 248 1 037 ✅ - 2 946 67 💤 - 302 0 ❌ ±0

Results for commit 5560ff0. ± Comparison against base commit 073eab9.

This pull request removes 3248 tests.

tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_lambda_dynamodb
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_opensearch_crud
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_search_books
tests.aws.scenario.bookstore.test_bookstore.TestBookstoreApplication ‑ test_setup
tests.aws.scenario.kinesis_firehose.test_kinesis_firehose.TestKinesisFirehoseScenario ‑ test_kinesis_firehose_s3
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_destination_sns
tests.aws.scenario.lambda_destination.test_lambda_destination_scenario.TestLambdaDestinationScenario ‑ test_infra
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_prefill_dynamodb_table
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input0-SUCCEEDED]
tests.aws.scenario.loan_broker.test_loan_broker.TestLoanBrokerScenario ‑ test_stepfunctions_input_recipient_list[step_function_input1-SUCCEEDED]
…

♻️ This comment has been updated with latest results.

cloutierMat

Super impressive work! I really like the solution you came up with to facilitate snapshotting the logs.

I love how we can reuse the handler chain for this! 👍

cloutierMat · 2025-04-11T17:28:24Z

localstack-core/localstack/services/apigateway/next_gen/execute_api/test_invoke.py

+    # TODO: funny enough, in AWS for the `endpoint_response_headers` in AWS_PROXY, they log the response headers from
+    #  lambda HTTP Invoke call even though we use the headers from the lambda response itself


😢 Never stop being surprised!

Yeah this is kinda weird 😄 somewhat leaking implementation details

cloutierMat · 2025-04-11T17:30:28Z

localstack-core/localstack/services/apigateway/next_gen/execute_api/test_invoke.py

+def create_test_chain() -> HandlerChain[RestApiInvocationContext]:
+    return HandlerChain(
+        request_handlers=[
+            handlers.method_request_handler,
+            handlers.integration_request_handler,
+            handlers.integration_handler,
+            handlers.integration_response_handler,
+            handlers.method_response_handler,
+        ],
+        exception_handlers=[
+            handlers.gateway_exception_handler,
+        ],
+    )


So clean! This really makes that work we did shine bright!

And it was so nice to actually be able to only pick a few handlers that we needed, skipping the parsing and routing of the request 👌

I mean you could add them all, and have a if is_test_invoke in them to produce a different result! 🤣

cloutierMat · 2025-04-11T17:32:13Z

localstack-core/localstack/services/apigateway/next_gen/execute_api/test_invoke.py

+def _fix_headers(headers: Headers) -> Headers:
+    headers.remove("Content-Length")
+
+    return headers


nit: this feels a bit out of place in the middle of the file 🫤 Maybe we could move it up with the _dump_headers util?

yeah agreed, it did multiple things before like adding the trace-id, but it looked like MOCK wasn't adding it... so I've just added the removal of Content-Length straight in the run method now. If we ever need to add more, we'll move it to its own function?

cloutierMat · 2025-04-11T17:34:46Z

tests/aws/services/apigateway/apigateway_fixtures.py

Thanks for the cleanup! 🙏

cloutierMat · 2025-04-11T17:39:18Z

tests/aws/services/apigateway/test_apigateway_basic.py

+        def _transform_log(_log: str) -> dict[str, str]:
+            return {f"line{index:02d}": line for index, line in enumerate(_log.split("\n"))}


Nice attention! It will be much easier to see the error when it arises! 🙏

bentsku

Thanks a lot for the review! I've just realized that I hadn't re-ran the test against AWS after fixing some regex shenanigans, I'll fix it and merge. This was a fun one 😄

bentsku · 2025-04-11T17:49:54Z

localstack-core/localstack/services/apigateway/next_gen/execute_api/test_invoke.py

+    # TODO: funny enough, in AWS for the `endpoint_response_headers` in AWS_PROXY, they log the response headers from
+    #  lambda HTTP Invoke call even though we use the headers from the lambda response itself


Yeah this is kinda weird 😄 somewhat leaking implementation details

bentsku · 2025-04-11T17:50:20Z

localstack-core/localstack/services/apigateway/next_gen/execute_api/test_invoke.py

+def create_test_chain() -> HandlerChain[RestApiInvocationContext]:
+    return HandlerChain(
+        request_handlers=[
+            handlers.method_request_handler,
+            handlers.integration_request_handler,
+            handlers.integration_handler,
+            handlers.integration_response_handler,
+            handlers.method_response_handler,
+        ],
+        exception_handlers=[
+            handlers.gateway_exception_handler,
+        ],
+    )


And it was so nice to actually be able to only pick a few handlers that we needed, skipping the parsing and routing of the request 👌

bentsku · 2025-04-11T17:51:19Z

localstack-core/localstack/services/apigateway/next_gen/execute_api/test_invoke.py

+def _fix_headers(headers: Headers) -> Headers:
+    headers.remove("Content-Length")
+
+    return headers


yeah agreed, it did multiple things before like adding the trace-id, but it looked like MOCK wasn't adding it... so I've just added the removal of Content-Length straight in the run method now. If we ever need to add more, we'll move it to its own function?

bentsku added 2 commits April 10, 2025 21:45

WIP: migrate TestInvoke to v2

3fa9dd6

implement TestInvokeMethod

b10d593

bentsku added aws:apigateway Amazon API Gateway semver: patch Non-breaking changes which can be included in patch releases labels Apr 10, 2025

bentsku added this to the 4.4 milestone Apr 10, 2025

bentsku self-assigned this Apr 10, 2025

fix MOCK & fixture

6421bf2

bentsku force-pushed the apigw-test-invoke branch from 836bfdb to 6421bf2 Compare April 10, 2025 22:56

bentsku marked this pull request as ready for review April 11, 2025 08:45

bentsku requested a review from cloutierMat as a code owner April 11, 2025 08:45

cloutierMat approved these changes Apr 11, 2025

View reviewed changes

address nits

5a6da99

bentsku commented Apr 11, 2025

View reviewed changes

add missing transformers

5560ff0

bentsku merged commit a1eec50 into master Apr 11, 2025
31 checks passed

bentsku deleted the apigw-test-invoke branch April 11, 2025 19:14

		# TODO: funny enough, in AWS for the `endpoint_response_headers` in AWS_PROXY, they log the response headers from
		# lambda HTTP Invoke call even though we use the headers from the lambda response itself

		def _transform_log(_log: str) -> dict[str, str]:
		return {f"line{index:02d}": line for index, line in enumerate(_log.split("\n"))}

Uh oh!

APIGW: migrate TestInvokeMethod to NextGen #12514

APIGW: migrate TestInvokeMethod to NextGen #12514

Uh oh!

Conversation

bentsku commented Apr 10, 2025

Motivation

Changes

Uh oh!

github-actions bot commented Apr 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

LocalStack Community integration with Pro

Uh oh!

cloutierMat left a comment

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!

Choose a reason for hiding this comment

Uh oh!

bentsku left a comment

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!

Uh oh!

Uh oh!

github-actions bot commented Apr 10, 2025 •

edited

Loading