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

test(s2n-quic-platform): drain tx packets before closing host #2106

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

camshaft
Copy link
Contributor

@camshaft camshaft commented Jan 31, 2024

Description of changes:

The testing environment currently closes a socket on drop. This means that any outstanding packets in the TX queue are not submitted.

This change fixes that by marking the socket as closed and draining it. Once empty, the socket queue is removed from the network.

Call-outs:

I've also added a small function to know if the caller is running inside of a test environment or not.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@camshaft camshaft force-pushed the camshaft/testing-io-tx-drain branch from 495b2c1 to c62e439 Compare February 7, 2024 08:49
@camshaft camshaft marked this pull request as ready for review February 7, 2024 09:53
@WesleyRosenblum WesleyRosenblum self-requested a review February 8, 2024 21:08
@camshaft camshaft force-pushed the camshaft/testing-io-tx-drain branch from c62e439 to 14e7156 Compare February 8, 2024 22:05
@camshaft camshaft force-pushed the camshaft/testing-io-tx-drain branch from 14e7156 to 29e8b5f Compare February 8, 2024 23:09
@@ -23,6 +23,11 @@ pub use time::now;

pub use bach::task::{self, primary, spawn};

// returns `true` if the caller is being executed in a testing environment
pub fn is_in_env() -> bool {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure I understand the name, why not something like is_testing_env?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i was thinking testing was already in the module path name (io::testing::is_in_env()) so it seemed a bit redundant. But I don't have any strong opinions on the name.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah, yeah thats fine then

@camshaft camshaft merged commit 9b543eb into main Feb 9, 2024
120 checks passed
@camshaft camshaft deleted the camshaft/testing-io-tx-drain branch February 9, 2024 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants