Skip to content
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

Strict concurrency for NIOHTTP1Tests #3169

Merged
merged 7 commits into from
Apr 7, 2025
Merged

Conversation

glbrntt
Copy link
Contributor

@glbrntt glbrntt commented Apr 2, 2025

No description provided.

@glbrntt glbrntt added the semver/none No version bump required. label Apr 2, 2025
@rnro rnro self-requested a review April 2, 2025 10:12
@rnro
Copy link
Contributor

rnro commented Apr 2, 2025

It looks like there are still outstanding concurrency issues

/swift-nio/Tests/NIOHTTP1Tests/TestUtils.swift:705:52: error: type 'FulfillOnFirstEventHandler' does not conform to the 'Sendable' protocol
441 | ///
442 | /// - Note: Once this is used more widely and shows value, we might want to put it into `NIOTestUtils`.
443 | final class FulfillOnFirstEventHandler: ChannelDuplexHandler {
    |             `- note: class 'FulfillOnFirstEventHandler' does not conform to the 'Sendable' protocol
444 |     typealias InboundIn = Any
445 |     typealias OutboundIn = Any
    :
703 |                     channel
704 |                 }.cascade(to: tcpAcceptedChannel)
705 |                 return channel.pipeline.addHandler(FulfillOnFirstEventHandler(channelActivePromise: accepted))
    |                                                    `- error: type 'FulfillOnFirstEventHandler' does not conform to the 'Sendable' protocol
706 |             }
707 |             .bind(to: bindTarget)

@glbrntt
Copy link
Contributor Author

glbrntt commented Apr 2, 2025

It looks like there are still outstanding concurrency issues

/swift-nio/Tests/NIOHTTP1Tests/TestUtils.swift:705:52: error: type 'FulfillOnFirstEventHandler' does not conform to the 'Sendable' protocol
441 | ///
442 | /// - Note: Once this is used more widely and shows value, we might want to put it into `NIOTestUtils`.
443 | final class FulfillOnFirstEventHandler: ChannelDuplexHandler {
    |             `- note: class 'FulfillOnFirstEventHandler' does not conform to the 'Sendable' protocol
444 |     typealias InboundIn = Any
445 |     typealias OutboundIn = Any
    :
703 |                     channel
704 |                 }.cascade(to: tcpAcceptedChannel)
705 |                 return channel.pipeline.addHandler(FulfillOnFirstEventHandler(channelActivePromise: accepted))
    |                                                    `- error: type 'FulfillOnFirstEventHandler' does not conform to the 'Sendable' protocol
706 |             }
707 |             .bind(to: bindTarget)

Oh, this is symlinked in from NIOPosixTests which I've been working on in a separate commit. I didn't retest locally when I broke things up per target which is why I missed this.

@glbrntt glbrntt requested a review from gjcairo April 2, 2025 18:28
@glbrntt glbrntt merged commit 202f987 into apple:main Apr 7, 2025
39 of 41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver/none No version bump required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants