Problem: config utils tests are disabled.#2402
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2402 +/- ##
==========================================
+ Coverage 86.83% 88.12% +1.28%
==========================================
Files 38 38
Lines 2173 2173
==========================================
+ Hits 1887 1915 +28
+ Misses 286 258 -28 |
tests/test_config_utils.py
Outdated
| @@ -185,12 +169,7 @@ def test_autoconfigure_read_both_from_file_and_env(monkeypatch, request, ssl_con | |||
| 'BIGCHAINDB_WSSERVER_ADVERTISED_HOST': WSSERVER_ADVERTISED_HOST, | |||
| 'BIGCHAINDB_WSSERVER_ADVERTISED_PORT': WSSERVER_ADVERTISED_PORT, | |||
| 'BIGCHAINDB_KEYRING': KEYRING, | |||
There was a problem hiding this comment.
Is KEYRING still being used?
| 'BIGCHAINDB_DATABASE_CRLFILE': ssl_context.crl, | ||
| 'BIGCHAINDB_DATABASE_CERTFILE': ssl_context.cert, | ||
| 'BIGCHAINDB_DATABASE_KEYFILE': ssl_context.key, | ||
| 'BIGCHAINDB_DATABASE_KEYFILE_PASSPHRASE': None}) |
There was a problem hiding this comment.
Are we no longer supporting ssl for MongoDB?
There was a problem hiding this comment.
I removed it by mistake. It's back now.
ldmberman
force-pushed
the
reenable-config-utils-tests
branch
from
e935f52 to
ffd1b7b
Compare
| assert bigchaindb.config['CONFIGURED'] is True | ||
|
|
||
|
|
||
| def test_bigchain_instance_raises_when_not_configured(request, monkeypatch): |
There was a problem hiding this comment.
I couldn't understand why don't we need this test anymore?
There was a problem hiding this comment.
Where do you expect ConfigurationError to come from? From the first glance, without a config assigned, it will fail with a different exception. Also, the test setup assigns a config so it does not work either way.
There was a problem hiding this comment.
It seems that this test wasn't of much help before. I guess we can do without it then
There was a problem hiding this comment.
It seems like the intent of the test is to make sure we fail in a controlled fashion if the config gets dropped in some way. Couldn't we modify it to use a patched autoconfigure with force=True to set a bad config and produce the error that way?
There was a problem hiding this comment.
Currently, the function does not raise ConfigurationError in case the config is not set or empty. The contract that was tested here is lost.
Solution: enable them back, cleanup no longer relevant parts.
ldmberman
force-pushed
the
reenable-config-utils-tests
branch
from
ffd1b7b to
884f458
Compare
4 participants
Solution: enable them back, cleanup no longer relevant parts.