chore: remove NIO from tests #903
Merged
Conversation
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
…erver Replaces NIO-based test servers with a new HttpTestServer implemented using Foundation and POSIX sockets, eliminating the need for swift-nio and related packages in test targets. Updates all affected test files to use the new server and refactors test logic for compatibility. This simplifies test dependencies and improves portability.
Consolidates two duplicate HTTP test server implementations into a single shared utility that can be used by both OTLP exporter tests and URLSession instrumentation tests. Changes: - Create unified HttpTestServer in Tests/Shared/TestUtils that combines functionality from both previous implementations - Uses POSIX sockets directly, maintaining independence from external dependencies (no NIO, Swifter, etc.) - Supports OTLP test requirements: request recording, protobuf binary payloads, header verification - Supports URLSession test requirements: path-based responses, success/error callbacks, configuration options - Add SharedTestUtils target to Package.swift for shared test utilities - Update both test targets to depend on SharedTestUtils - Remove duplicate implementations from ExportersTests and InstrumentationTests - Update test files to import SharedTestUtils and use consistent API Benefits: - Eliminates code duplication between test suites - Provides consistent HTTP mocking functionality - Makes it easier to maintain and extend test infrastructure - All OTLP HTTP exporter tests (16) passing successfully Note: URLSession tests have 4 pre-existing failures in error handling that are unrelated to this refactoring.
Added logic to HttpTestServer to properly handle requests to paths starting with /error when no config is provided, simulating a network error by closing the connection without a response.
The Darwin import was not used in HttpTestServer.swift
- Add conditional imports for Glibc/Musl to support Linux builds - Replace macOS-specific CFSwapInt16BigToHost with portable ntohs for byte order conversion - Fix shutdown() calls to use platform-specific namespaces (Darwin/Glibc/Musl) - Fix minor code warnings (unused variable, boolean test) The HTTP test server now compiles and runs correctly on both macOS and Linux platforms, maintaining full backward compatibility while enabling cross-platform support. This change is required for the test suite to pass in Linux CI environments.
martin308
force-pushed
the
martin308/remove-nio-from-tests
branch
from
2d8e446 to
114edc2
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #903 +/- ##
==========================================
- Coverage 67.89% 61.67% -6.23%
==========================================
Files 344 82 -262
Lines 15169 5724 -9445
==========================================
- Hits 10299 3530 -6769
+ Misses 4870 2194 -2676 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Fix SOCK_STREAM type conversion on Linux (cast __socket_type to Int32) - Fix SHUT_RDWR type conversion on Linux (cast Int to Int32) These changes ensure proper type compatibility between macOS and Linux socket APIs.
- Add conditional compilation for Darwin vs Linux socket() call - macOS uses SOCK_STREAM directly, Linux needs Int32(SOCK_STREAM.rawValue) - Silence unused result warnings for fcntl and withCString calls - Maintain full compatibility with both OTLP and URLSession tests
martin308
requested review from
nachoBonafonte,
bryce-b,
vvydier and
ArielDemarco
as code owners
ArielDemarco
approved these changes
Sep 10, 2025
nachoBonafonte
approved these changes
Sep 10, 2025
- Ensures clean state when restarting server - Prevents potential memory leaks from accumulated requests - Maintains consistency with initialization state
|
@ArielDemarco @nachoBonafonte I still see some NIO looking bits (imports and a usage of
Shouldn't they fail to build now the dependencies have been removed from the Package.swift file? |
williazz
reviewed
Sep 29, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This pull request refactors test dependencies and utilities to simplify the test infrastructure and remove direct dependencies on
swift-niotest utilities. The main change is the introduction of a sharedHttpTestServerutility, which is now used across multiple test targets. This reduces reliance on low-level NIO test servers and streamlines test setup and teardown.Test Infrastructure Refactoring:
SharedTestUtilstarget inPackage.swiftto provide shared testing utilities for multiple test targets.OpenTelemetryProtocolExporterTests,URLSessionInstrumentationTests) to depend onSharedTestUtilsinstead of directly depending onswift-niotest products. [1] [2]HttpTestServerimplementation fromTests/InstrumentationTests/URLSessionTests/Utils/HttpTestServer.swift, consolidating server utilities intoSharedTestUtils.Test Code Simplification and Modernization:
NIOHTTP1TestServerand related NIO constructs in test files with the newHttpTestServerfromSharedTestUtils, simplifying server setup and teardown logic. [1] [2] [3] [4]String(decoding:body,as:UTF8.self)instead of NIOByteBuffer). [1] [2] [3] [4]These changes collectively improve the maintainability and clarity of the test suite by centralizing test utilities and reducing external dependencies.