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.

Sign up for GitHub

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

Jump to bottom

Fix a visual glitch in test_cmp_http.t #27144

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Fix a visual glitch in test_cmp_http.t #27144

wants to merge 2 commits into from
+6 −1

Conversation

bernd-edlinger
Copy link
Member

Kill the shell process after the Mock server is running, to prevent the shell from printing an error message when the Mock server is finally killed.

@bernd-edlinger
Fix a visual glitch in test_cmp_http.t
Loading
Loading status checks…
d5a2046 
Kill the shell process after the Mock server is running,
to prevent the shell from printing an error message when
the Mock server is finally killed.
@bernd-edlinger bernd-edlinger added branch: master Merge to master branch branch: 3.2 Merge to openssl-3.2 branch: 3.3 Merge to openssl-3.3 branch: 3.4 Merge to openssl-3.4 branch: 3.5 Merge to openssl-3.5 labels Mar 24, 2025
paulidale
paulidale previously approved these changes Mar 24, 2025
View reviewed changes
@paulidale paulidale added the approval: review pending This pull request needs review by a committer label Mar 24, 2025
@bernd-edlinger
Copy link
Member Author

all macos-14/15 test runs show this ugly effect:

90-test_sysdefault.t .................... ok

sh: line 1: 21846 Killed: 9               ../../../../util/wrap.pl ../../../../apps/openssl cmp -config server.cnf 2> ../../../../test-runs/test_cmp_http/Mock_server_STDERR.txt80-test_cmp_http.t ...................... ok
80-test_ssl_old.t ....................... ok

that is fixed by this.

t8m
t8m previously approved these changes Mar 25, 2025
View reviewed changes
@t8m t8m added triaged: bug The issue/pr is/fixes a bug tests: present The PR has suitable tests present 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 Mar 25, 2025
@bernd-edlinger bernd-edlinger mentioned this pull request Mar 25, 2025
Open
@tmshort
Copy link
Contributor

tmshort commented Mar 25, 2025

I think the 3.0 backport code (#27158) is clearer than this code.

@openssl-machine
Copy link
Collaborator

24 hours has passed since 'approval: done' was set, but as this PR has been updated in that time the label 'approval: ready to merge' is not being automatically set. Please review the updates and set the label manually.

@bernd-edlinger bernd-edlinger dismissed stale reviews from t8m and paulidale via aae52b7 March 26, 2025 13:47
@bernd-edlinger
Copy link
Member Author

OK, I have updated the PR as requested. Please re-confirm.

@t8m t8m requested review from paulidale and tmshort March 26, 2025 14:46
@bernd-edlinger bernd-edlinger requested a review from DDvO March 30, 2025 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@t8m t8m

@paulidale paulidale

@tmshort tmshort

@DDvO DDvO

At least 2 approving reviews are required to merge this pull request.

Assignees
No one assigned
Labels
approval: done This pull request has the required number of approvals branch: master Merge to master branch branch: 3.2 Merge to openssl-3.2 branch: 3.3 Merge to openssl-3.3 branch: 3.4 Merge to openssl-3.4 branch: 3.5 Merge to openssl-3.5 tests: present The PR has suitable tests present triaged: bug The issue/pr is/fixes a bug
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@bernd-edlinger @tmshort @openssl-machine @t8m @paulidale