-
Notifications
You must be signed in to change notification settings - Fork 116
add support for LOCAL_LAMBDA_PORT #557
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
ad5c428
add support for LOCAL_LAMBDA_PORT
sebsto 7defd27
change (c) year
sebsto fcd7951
fix typo
sebsto 44a1258
add an env variable for host to be consistent
sebsto c6c1f91
Ensure all tests using the Runtime or RuntimeClient are serialized
sebsto c88eff0
set mock server on port 0
sebsto 48b1571
remove .serialized when not needed
sebsto 2216a5c
Group tests using `LambdaRuntime.run()` in teh same `.serialized` suite
sebsto 0a8a764
Merge branch 'main' into sebsto/local_port
sebsto File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,94 @@ | ||
//===----------------------------------------------------------------------===// | ||
// | ||
// This source file is part of the SwiftAWSLambdaRuntime open source project | ||
// | ||
// Copyright (c) 2025 Apple Inc. and the SwiftAWSLambdaRuntime project authors | ||
// Licensed under Apache License v2.0 | ||
// | ||
// See LICENSE.txt for license information | ||
// See CONTRIBUTORS.txt for the list of SwiftAWSLambdaRuntime project authors | ||
// | ||
// SPDX-License-Identifier: Apache-2.0 | ||
// | ||
//===----------------------------------------------------------------------===// | ||
|
||
import Logging | ||
import NIOCore | ||
import NIOPosix | ||
import Testing | ||
|
||
@testable import AWSLambdaRuntime | ||
|
||
extension LambdaRuntimeTests { | ||
|
||
@Test("Local server respects LOCAL_LAMBDA_PORT environment variable") | ||
func testLocalServerCustomPort() async throws { | ||
let customPort = 8080 | ||
|
||
// Set environment variable | ||
setenv("LOCAL_LAMBDA_PORT", "\(customPort)", 1) | ||
defer { unsetenv("LOCAL_LAMBDA_PORT") } | ||
|
||
let result = try? await withThrowingTaskGroup(of: Bool.self) { group in | ||
|
||
// start a local lambda + local server on custom port | ||
group.addTask { | ||
// Create a simple handler | ||
struct TestHandler: StreamingLambdaHandler { | ||
func handle( | ||
_ event: ByteBuffer, | ||
responseWriter: some LambdaResponseStreamWriter, | ||
context: LambdaContext | ||
) async throws { | ||
try await responseWriter.write(ByteBuffer(string: "test")) | ||
try await responseWriter.finish() | ||
} | ||
} | ||
|
||
// create the Lambda Runtime | ||
let runtime = LambdaRuntime( | ||
handler: TestHandler(), | ||
logger: Logger(label: "test", factory: { _ in SwiftLogNoOpLogHandler() }) | ||
) | ||
|
||
// Start runtime | ||
try await runtime._run() | ||
|
||
// we reach this line when the group is cancelled | ||
return false | ||
} | ||
|
||
// start a client to check if something responds on the custom port | ||
group.addTask { | ||
// Give server time to start | ||
try await Task.sleep(for: .milliseconds(100)) | ||
|
||
// Verify server is listening on custom port | ||
return try await isPortResponding(host: "127.0.0.1", port: customPort) | ||
} | ||
|
||
let first = try await group.next() | ||
group.cancelAll() | ||
return first ?? false | ||
|
||
} | ||
|
||
#expect(result == true) | ||
} | ||
|
||
private func isPortResponding(host: String, port: Int) async throws -> Bool { | ||
let group = MultiThreadedEventLoopGroup(numberOfThreads: 1) | ||
|
||
let bootstrap = ClientBootstrap(group: group) | ||
|
||
do { | ||
let channel = try await bootstrap.connect(host: host, port: port).get() | ||
try await channel.close().get() | ||
try await group.shutdownGracefully() | ||
return true | ||
} catch { | ||
try await group.shutdownGracefully() | ||
sebsto marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return false | ||
} | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Given these tests are running in parallel, there is a chance the LOCAL_LAMBDA_PORT environment var setting will affect other tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adam-fowler good point. I aggressively added
.serialized
on all tests using the server or the runtime clientThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that might only run tests serially within each suite so two suites can still run in parallel, even though the tests internally run serially. Might be worth asking swift-testing folks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adam-fowler During the tests, we let the MockServer bind on any available port (port = 0) and share its actual port number with the runtime client. There is no risk of conflict.
See
swift-aws-lambda-runtime/Tests/AWSLambdaRuntimeTests/MockLambdaServer.swift
Line 28 in ec28c96
I kept the
.serialized
only on the new test in case someone adds another test to this suite later on.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The problem isn't the fact you might have multiple servers binding to the same address (although that is a problem and am surprised it hasn't reared it head before). The problem is the environment variable is being set when other tests could be running and could affect those tests.
We can leave as is, but I think we will get the occasional test failure as multiple tests inherit the environment variable and bind to the same address.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adam-fowler
In general, the unit tests don't use the local server. They use a MockServer that uses a random port number to avoid conflicts.
There are three groups of tests that use the real server :
LambdaRuntimeTests.swift
LambdaRuntime+ServiceLifecycle.swift
LambdaLocalServerTests.swift
(this PR)I will add a commit to group them in the same
.serialized
suite.Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adam-fowler wdyt ? 2216a5c