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

QUIC: Run multistream tests in blocking mode as well #21827

Closed
wants to merge 7 commits into from

Conversation

hlandau
Copy link
Member

@hlandau hlandau commented Aug 24, 2023

This expands the multistream test to run everything in both blocking and non-blocking mode using the same set of scripts. This should dramatically increase our test coverage for our APIs in blocking mode.

Internally this works by spinning up an extra thread to tick the TSERVER so that we can enter a blocking call in the main thread.

Built on the WAIT_PEER PR.

@hlandau hlandau added branch: master Merge to master branch approval: review pending This pull request needs review by a committer approval: otc review pending This pull request needs review by an OTC member triaged: feature The issue/pr requests/adds a feature tests: present The PR has suitable tests present labels Aug 24, 2023
@hlandau hlandau self-assigned this Aug 24, 2023
@hlandau hlandau force-pushed the quic-test-blocking branch 2 times, most recently from 94b8273 to 4a3f1f2 Compare August 24, 2023 10:22
@t8m
Copy link
Member

t8m commented Aug 24, 2023

CI still relevant

@hlandau
Copy link
Member Author

hlandau commented Aug 24, 2023

Yeah, I'm investigating.

@t8m
Copy link
Member

t8m commented Aug 24, 2023

Hmm... it seems it still doesn't work properly on Windows.

@hlandau
Copy link
Member Author

hlandau commented Aug 25, 2023

This is due to our Windows code not having an implementation of ossl_crypto_condvar_broadcast. I'm working on a fix.

@trixieboyle

This comment was marked as spam.

@github-actions github-actions bot added the severity: fips change The pull request changes FIPS provider sources label Aug 29, 2023
@hlandau
Copy link
Member Author

hlandau commented Aug 30, 2023

Updated and rebased, CI passing, ready for review.

This adds a new Win32 pre-Vista condition variable emulation algorithm, and tests on Windows are now passing.

@mattcaswell
Copy link
Member

Needs another rebase

@hlandau
Copy link
Member Author

hlandau commented Sep 4, 2023

Rebased

@t8m t8m closed this Sep 4, 2023
@t8m t8m reopened this Sep 4, 2023
@mattcaswell
Copy link
Member

Hmmm....is CI failure relevant?

@hlandau
Copy link
Member Author

hlandau commented Sep 4, 2023

Hmmm....is CI failure relevant?

It doesn't look like it.

Updated.

@t8m t8m removed the approval: otc review pending This pull request needs review by an OTC member label Sep 4, 2023
@mattcaswell mattcaswell added approval: done This pull request has the required number of approvals and removed approval: review pending This pull request needs review by a committer labels Sep 5, 2023
@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but this PR has failing CI tests. Once the tests pass it will get moved to 'approval: ready to merge' automatically, alternatively please review and set the label manually.

@hlandau
Copy link
Member Author

hlandau commented Sep 6, 2023

Merged to master. Thanks for the reviews.

@hlandau hlandau closed this Sep 6, 2023
openssl-machine pushed a commit that referenced this pull request Sep 6, 2023
…ing modes

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #21827)
openssl-machine pushed a commit that referenced this pull request Sep 6, 2023
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #21827)
openssl-machine pushed a commit that referenced this pull request Sep 6, 2023
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #21827)
openssl-machine pushed a commit that referenced this pull request Sep 6, 2023
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #21827)
openssl-machine pushed a commit that referenced this pull request Sep 6, 2023
Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #21827)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approval: done This pull request has the required number of approvals branch: master Merge to master branch severity: fips change The pull request changes FIPS provider sources tests: present The PR has suitable tests present triaged: feature The issue/pr requests/adds a feature
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

5 participants