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: Display the correct shared library name #15776

Closed

Conversation

levitte
Copy link
Member

@levitte levitte commented Jun 15, 2021

In test/recipes/01-test_symbol_presence.t

In test/recipes/01-test_symbol_presence.t
@levitte levitte added branch: master Merge to master branch approval: review pending This pull request needs review by a committer labels Jun 15, 2021
mattcaswell
mattcaswell previously approved these changes Jun 15, 2021
@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 Jun 15, 2021
@t8m
Copy link
Member

t8m commented Jun 15, 2021

Please note that MacOS build regressed due to the earlier test/recipes/01-test_symbol_presence.t fix.

@mattcaswell
Copy link
Member

Should we just skip this test on macos? Its a developer thing really

It renames symbols, so we can a false negative
@levitte
Copy link
Member Author

levitte commented Jun 15, 2021

Should we just skip this test on macos? Its a developer thing really

I pushed an extra commit that does exactly that

@levitte levitte added the severity: urgent Fixes an urgent issue (exempt from 24h grace period) label Jun 15, 2021
@levitte
Copy link
Member Author

levitte commented Jun 15, 2021

Because of the regression, I made this urgent

@levitte levitte dismissed mattcaswell’s stale review June 15, 2021 16:52

Needs reapproval

@levitte levitte added approval: review pending This pull request needs review by a committer and removed approval: done This pull request has the required number of approvals labels Jun 15, 2021
Copy link
Member

@mattcaswell mattcaswell left a comment

Choose a reason for hiding this comment

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

Approved. Agreed urgent. The Windows CI failures are not relevant (a power failure knocked out the nasm website...its just come back up).

@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 Jun 15, 2021
@paulidale
Copy link
Contributor

Merged to master.

@paulidale paulidale closed this Jun 15, 2021
openssl-machine pushed a commit that referenced this pull request Jun 15, 2021
In test/recipes/01-test_symbol_presence.t

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #15776)
openssl-machine pushed a commit that referenced this pull request Jun 15, 2021
It renames symbols, so we can a false negative

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from #15776)
@levitte levitte deleted the fix-test_symbol_presence-20210615-2 branch June 16, 2021 15:17
devnexen pushed a commit to devnexen/openssl that referenced this pull request Jul 7, 2021
In test/recipes/01-test_symbol_presence.t

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from openssl#15776)
devnexen pushed a commit to devnexen/openssl that referenced this pull request Jul 7, 2021
It renames symbols, so we can a false negative

Reviewed-by: Matt Caswell <matt@openssl.org>
Reviewed-by: Paul Dale <pauli@openssl.org>
(Merged from openssl#15776)
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: urgent Fixes an urgent issue (exempt from 24h grace period)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants