Skip to content

Test apps s_time with new -testmode option#31192

Open
bukka wants to merge 1 commit into
openssl:masterfrom
bukka:apps-test-stime
Open

Test apps s_time with new -testmode option#31192
bukka wants to merge 1 commit into
openssl:masterfrom
bukka:apps-test-stime

Conversation

@bukka
Copy link
Copy Markdown
Member

@bukka bukka commented May 15, 2026

This tests apps s_time that is currently completely untested.

It is composed of 4 tests and runs around 8s in total (not sure how to make it quicker as it's using min time). The s_time is currently not tested at all so it will improve the apps coverage a bit.

EDIT: A new -testmode param was added so the test takes only 800 ms which should be acceptable.

@bukka bukka marked this pull request as draft May 15, 2026 12:48
@bukka bukka force-pushed the apps-test-stime branch from c116cfb to 354b127 Compare May 15, 2026 13:37
@openssl-machine openssl-machine added the approval: review pending This pull request needs review by a committer label May 15, 2026
@bukka bukka force-pushed the apps-test-stime branch 6 times, most recently from 222be25 to a223559 Compare May 15, 2026 16:04
@bukka bukka self-assigned this May 15, 2026
@bukka bukka moved this to In Progress in Development Board May 15, 2026
@bukka bukka moved this from In Progress to Waiting Review in Development Board May 15, 2026
@bukka bukka marked this pull request as ready for review May 15, 2026 16:59
@bob-beck
Copy link
Copy Markdown
Contributor

s_time is really just a connection counter and averager, while ok, sure, it improves test coverage I'm not sure I want 8 seconds of extra time expended on every time I run the tests to verify this. I run tests a lot.

@paulidale
Copy link
Copy Markdown
Contributor

The time this takes will be in parallel to other tasks and shouldn't increase the overall run time.
Except on single threaded test runs which ought to be uncommon.

@bukka
Copy link
Copy Markdown
Member Author

bukka commented May 18, 2026

Yeah it will run in parallel so real slow down won't be that big.

The main reason for the test is that the tool is not going to regress easily and testing multiple session reuse might be useful as well I guess - think it is already tested but usually just with few reconnections. I would need to check that part if it brings anything new though. Still having some tools with 0 coverage is not ideal (although not sure if it's worth it to spend time on things like srp just to increase coverage).

@t8m
Copy link
Copy Markdown
Member

t8m commented May 19, 2026

We have special-cased similar test use case in openssl speed by adding an option. We could for example special-case -time 0 as do just a couple iterations. This is for master branch only anyway.

@t8m t8m added branch: master Applies to master branch triaged: feature The issue/pr requests/adds a feature tests: present The PR has suitable tests present labels May 19, 2026
@t8m t8m moved this from Waiting Review to In Progress in Development Board May 21, 2026
@bukka bukka force-pushed the apps-test-stime branch from a223559 to 54a6f3f Compare May 29, 2026 20:05
@bukka bukka changed the title Test apps s_time Test apps s_time with new -testmode param May 29, 2026
@bukka bukka force-pushed the apps-test-stime branch from 54a6f3f to 2b1ecd2 Compare May 29, 2026 20:09
@bukka bukka changed the title Test apps s_time with new -testmode param Test apps s_time with new -testmode option May 29, 2026
@bukka bukka force-pushed the apps-test-stime branch from 2b1ecd2 to 30f92e8 Compare May 29, 2026 20:10
Adds -testmode to s_time, mirroring the option in openssl speed.
It bypasses the -time window and runs a minimal number of iterations
(1 for new connections, 2 for session reuse).

Adds test_stime covering the new, reuse, and TLSv1.2/TLSv1.3 paths.
@bukka bukka force-pushed the apps-test-stime branch from 30f92e8 to 8835530 Compare May 29, 2026 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approval: review pending This pull request needs review by a committer branch: master Applies to master branch tests: present The PR has suitable tests present triaged: feature The issue/pr requests/adds a feature

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

5 participants