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

[fix] Add tests for produce and consume with TLS enabled #313

Merged
merged 2 commits into from
Apr 6, 2023

Conversation

massakam
Copy link
Contributor

@massakam massakam commented Apr 6, 2023

Motivation

There was a problem with the code just a little while ago where specifying an HTTPS URL as the serviceUrl caused a segfault. This has been fixed by #310, but since there were no tests for this issue yet, I added it.

In addition, it seemed that the place where -fvisibility=hidden should be added was wrong, so I fixed it.

Verifying this change

  • Make sure that the change passes the CI checks.

Documentation

  • doc-required
    (Your PR needs to update docs and you will update later)

  • doc-not-needed
    (Please explain why)

  • doc
    (Your PR contains doc changes)

  • doc-complete
    (Docs have been already added)

@shibd shibd requested a review from RobertIndie April 6, 2023 08:51
Copy link
Member

@shibd shibd left a comment

Choose a reason for hiding this comment

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

Sorry, Please wait for me to verify this parameter: -fvisibility=hidden

@shibd shibd merged commit 74461ca into apache:master Apr 6, 2023
@shibd shibd added this to the 1.9.0 milestone Apr 6, 2023
@massakam massakam deleted the fix-segfault branch April 6, 2023 10:16
shibd pushed a commit to streamnative/pulsar-client-node that referenced this pull request Apr 9, 2023
* Add tests for produce and consume with TLS enabled

* Add error message output

(cherry picked from commit 74461ca)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants