-
Notifications
You must be signed in to change notification settings - Fork 154
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
Add end to end tests using OpenSSL #193
Conversation
4299695
to
10db9a6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about doing like this? feature/e2e-openssl...feature/e2e-openssl-at-wat-patch
10db9a6
to
c734c03
Compare
Thanks for doing the job! I've already merged your changes into this branch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My patch commit is errored due to the commit message lint.
Could you update the commit message?
c734c03
to
ae365f2
Compare
This is pretty great, thanks so much for working on this! I'm wondering if we should go ahead and merge this, and then add ED25519 and PSK tests incrementally, or if we want to hold this up until the other two are implemented? If the former, we should probably drop |
I agree with merging this even if ED25519 and PSK tests are not implemented. We could update the E2E issue #11 to mark what has been done and what is missing. Having commented code int I am updating |
ae365f2
to
2f9d75c
Compare
I feel it's fine to keep it commented for now. |
I have uncommented PSK tests and skipped ED25519 tets |
3982a71
to
079f109
Compare
079f109
to
ce88a01
Compare
Current E2E test output:
We're getting there! Need to figure out if we care about that OpenSSL error though and if not how to silence it. |
3522807
to
19a7e52
Compare
Once #203 is merged OpenSSL errors should disappear. We could also add this, to force any future verification error make the test fail:
|
I like |
2279cf7
to
1b1f654
Compare
I'd prefer a squash, since it's essentially a single "feature". cc @Sean-Der could we allow for that ^^? Avoids an extra back-and-forth and re-approving the same thing. |
End to end tests are now executed in the folowing modes: - pion client + pion server - openssl client + pion server - pion client + openssl server OpenSSL is launched using `exec.Command` and therefore `openssl` command must be available before running these tests. Tests are enabled via `openssl` tag.
1b1f654
to
152c01e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
End to end tests are now executed in the folowing modes:
default
: pion client, pion serveropenssl_client
: openssl client, pion serveropenssl_server
: pion client, openssl serverOpenSSL is launched using
exec.Command
TODO: