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 failures without openssl-cli #9509

Closed
shigeki opened this issue Nov 8, 2016 · 9 comments
Closed

test failures without openssl-cli #9509

shigeki opened this issue Nov 8, 2016 · 9 comments
Labels
good first issue Issues that are suitable for first-time contributors. openssl Issues and PRs related to the OpenSSL dependency. test Issues and PRs related to the tests.

Comments

@shigeki
Copy link
Contributor

shigeki commented Nov 8, 2016

  • Version: v8.0.0-pre
  • Platform: all
  • Subsystem: test

When openssl-cli is not found, several tests are failed due to missing its check.

$ rm out/Release/openssl-cli
$ python tools/test.py --mode=release parallel -J
(snip)
[00:22|% 100|+ 1166|-   4]: Done

This issue is reserved to be fixed in the course of Code and Learn in Node Fest Tokyo 2016.

Ref: nodejs/code-and-learn#58

@shigeki shigeki added test Issues and PRs related to the tests. good first issue Issues that are suitable for first-time contributors. labels Nov 8, 2016
@bnoordhuis
Copy link
Member

Is that something that really needs fixing though? It only breaks when you actively sabotage it.

@shigeki
Copy link
Contributor Author

shigeki commented Nov 8, 2016

I think the tests would fail if Node is built with shared openssl library in which openssl-cli does not exist.

@bnoordhuis
Copy link
Member

I understand now. That's right, it won't get built. I'd probably make openssl-cli build unconditionally rather than trying to fix up the tests.

@shigeki
Copy link
Contributor Author

shigeki commented Nov 8, 2016

The common.opensslCli looks for external openssl command when shared library is used so that the test failure would happened in very limited case, as only libraries are installed or it does not have a path to the command.

So I think it is good to check via common.opensslCli such as other tests do now rather than to build openssl-cli even in shared library.

@mscdex mscdex added the openssl Issues and PRs related to the OpenSSL dependency. label Nov 8, 2016
@shigeki
Copy link
Contributor Author

shigeki commented Nov 18, 2016

No one worked in this issue so it is open to everyone.

@sotayamashita
Copy link
Contributor

sotayamashita commented Jan 25, 2017

I would like to work on this one. 💪
https://github.com/sotayamashita/node/tree/feature/test-openssl-cli

@sotayamashita
Copy link
Contributor

sotayamashita commented Feb 1, 2017

@shigeki I would like to ask a question. How to generate common/ which is required in test code.
Thanks in advance.

I miss understood and I figure it out now 😃

@sotayamashita
Copy link
Contributor

Landed in 5ffb7d7.

@shigeki Could you close this issue ?

@shigeki
Copy link
Contributor Author

shigeki commented Feb 14, 2017

@sotayamashita Thanks.

@shigeki shigeki closed this as completed Feb 14, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Issues that are suitable for first-time contributors. openssl Issues and PRs related to the OpenSSL dependency. test Issues and PRs related to the tests.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants