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

95-test_external_pyca_data/cryptography.py: only install for testing #3007

Closed
wants to merge 3 commits into from

Conversation

levitte
Copy link
Member

@levitte levitte commented Mar 21, 2017

Also, be less silent when installing, so possible errors are shown.

[extended tests]

Fixes #3005

@alex
Copy link
Contributor

alex commented Mar 21, 2017

Which of the builder's output should I look at?

@levitte
Copy link
Member Author

levitte commented Mar 21, 2017

Hold on, I just condensed .travis.yml to only contain builds that would only exercise test_external_pyca

@levitte
Copy link
Member Author

levitte commented Mar 21, 2017

So from now on, it's any build you can see...

@alex
Copy link
Contributor

alex commented Mar 21, 2017

I think you may have excluded too much, in the output I see "ok 1 # skip PYCA Cryptography not available"

@levitte
Copy link
Member Author

levitte commented Mar 21, 2017

Yeah, I noticed. I'm not sure what caused this, but I keep experimenting. I see further up in the log that the pyca-cryptography submodule is checked out, so pyca-cryptography/setup.py should be there...

@levitte levitte force-pushed the fix-test_pyca-20170321 branch 2 times, most recently from f6fcd40 to 71e78c8 Compare March 21, 2017 13:20
@levitte
Copy link
Member Author

levitte commented Mar 21, 2017

Finally got it right...

@alex
Copy link
Contributor

alex commented Mar 21, 2017

Ok, we now have a clear error message: https://travis-ci.org/openssl/openssl/builds/213413215#L808-L818

If you don't want to install all our weird dependencies for a full dev environment you can reply pip install -r dev-requirements.txt with pip install .[test], that should get everything you need just to run the tests.

@levitte
Copy link
Member Author

levitte commented Mar 21, 2017

Right now, I'm trying the other way around, to install libenchant-dev to see if that solves the issue. But if only pip-installing the test requirements is enough, I'm game as well!

@levitte
Copy link
Member Author

levitte commented Mar 21, 2017

I think I'll opt for simplicity, so pip install .[test] it is.

Time to make a nice patch of this PR and take it out of WIP.

@alex
Copy link
Contributor

alex commented Mar 21, 2017

👍

Also, be less silent when installing, so possible errors are shown.

[extended tests]

Fixes openssl#3005
@levitte levitte changed the title WIP: Make 95-test_external_pyca_data/cryptography.py less silent 95-test_external_pyca_data/cryptography.py: only install for testing Mar 21, 2017
@levitte
Copy link
Member Author

levitte commented Mar 21, 2017

Done. Review time.

@levitte
Copy link
Member Author

levitte commented Mar 21, 2017

.travis.yml is restore. In the current build, the xxxx.9 job is the one to look at.

@levitte
Copy link
Member Author

levitte commented Mar 21, 2017

Sorry, wrong, I meant the xxxx.7 job.

@levitte
Copy link
Member Author

levitte commented Mar 21, 2017

It seems I'm getting another type of error now:

Traceback (most recent call last):
  File "/home/travis/build/openssl/openssl/venv-pycrypto/local/lib/python2.7/site-packages/_pytest/config.py", line 362, in _importconftest
    mod = conftestpath.pyimport()
  File "/home/travis/build/openssl/openssl/venv-pycrypto/local/lib/python2.7/site-packages/py/_path/local.py", line 662, in pyimport
    __import__(modname)
  File "/home/travis/build/openssl/openssl/venv-pycrypto/local/lib/python2.7/site-packages/_pytest/assertion/rewrite.py", line 216, in load_module
    py.builtin.exec_(co, mod.__dict__)
  File "/home/travis/build/openssl/openssl/venv-pycrypto/local/lib/python2.7/site-packages/py/_builtin.py", line 221, in exec_
    exec2(obj, globals, locals)
  File "<string>", line 7, in exec2
  File "/home/travis/build/openssl/openssl/pyca-cryptography/tests/conftest.py", line 10, in <module>
    from cryptography.hazmat.backends.openssl import backend as openssl_backend
  File "/home/travis/build/openssl/openssl/pyca-cryptography/src/cryptography/hazmat/backends/openssl/__init__.py", line 7, in <module>
    from cryptography.hazmat.backends.openssl.backend import backend
  File "/home/travis/build/openssl/openssl/pyca-cryptography/src/cryptography/hazmat/backends/openssl/backend.py", line 49, in <module>
    from cryptography.hazmat.bindings._openssl import lib as _lib
ImportError: /home/travis/build/openssl/openssl/pyca-cryptography/src/cryptography/hazmat/bindings/_openssl.so: undefined symbol: __gcov_merge_add
ERROR: could not load /home/travis/build/openssl/openssl/pyca-cryptography/tests/conftest.py

That particular job is configured no-shared... @alex, that would never work with the pyca tests, would it? It seems we need to make a separate job for external tests...

@alex
Copy link
Contributor

alex commented Mar 21, 2017

It's definitely possibly to statically link a .a with pyca/cryptography, though I can't remember how off hand. /cc @reaperhulk

Some of the external tests do not run well with 'no-shared'
@levitte
Copy link
Member Author

levitte commented Mar 21, 2017

I find this reason enough to have a separate job for external tests...

@levitte
Copy link
Member Author

levitte commented Mar 21, 2017

Ok, it builds!

@mattcaswell mattcaswell added the approval: done This pull request has the required number of approvals label Mar 21, 2017
levitte added a commit that referenced this pull request Mar 21, 2017
Also, be less silent when installing, so possible errors are shown.

[extended tests]

Fixes #3005

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #3007)
levitte added a commit that referenced this pull request Mar 21, 2017
Some of the external tests do not run well with 'no-shared'

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #3007)
levitte added a commit that referenced this pull request Mar 21, 2017
[extended tests]

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from #3007)
@levitte
Copy link
Member Author

levitte commented Mar 21, 2017

Merged. 8c1054a to 34fffdb

@levitte levitte closed this Mar 21, 2017
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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants