Problem: tests in test_bigchain_api are not activated#2456
Problem: tests in test_bigchain_api are not activated#2456kansi merged 3 commits intobigchaindb:masterfrom
Conversation
Solution: remove or active tests
Codecov Report
@@ Coverage Diff @@
## master #2456 +/- ##
==========================================
+ Coverage 89.09% 89.13% +0.03%
==========================================
Files 41 41
Lines 2385 2384 -1
==========================================
Hits 2125 2125
+ Misses 260 259 -1 |
tests/db/test_bigchain_api.py
Outdated
| b.write_vote(vote) | ||
| b.store_bulk_transactions([signed_create_tx, signed_transfer_tx]) | ||
|
|
||
| sleep(1) |
There was a problem hiding this comment.
Should be no need for this sleep, let's remove it.
| class TestBigchainApi(object): | ||
|
|
||
| @pytest.mark.tendermint | ||
| def test_get_spent_with_double_inclusion_detected(self, b, alice): |
There was a problem hiding this comment.
Could you please explain why this test was removed?
There was a problem hiding this comment.
because we don't have a double inclusion anymore. The test was supposed to test the exception CriticalDoubleInclusion that was removed in #2415. Now with the changes, everything that happens in this test, happens in some other tests too, so we don't need it anymore. If you have an idea how to test a double inclusion I'll gladly rewrite it :)
There was a problem hiding this comment.
I think this test should just work as it is i.e. the test tries to insert two transactions with the same transaction ids so it would make sense to keep this test to ensure the transactions are uniquely indexed by their ids.
There was a problem hiding this comment.
We still tests the OperationError in another test:
bigchaindb/tests/db/test_bigchain_api.py
Line 47 in f127342
and the existing double spend errors in the test below:
bigchaindb/tests/db/test_bigchain_api.py
Line 16 in f127342
|
|
||
| @pytest.mark.bdb | ||
| @pytest.mark.tendermint | ||
| def test_transaction_unicode(b, alice): |
There was a problem hiding this comment.
I do not see any value in this test. What do you think about removing it?
There was a problem hiding this comment.
I don't know if it's valuable or not. But I do think it's good to check if unicode works. Maybe someone else has an opinion too
There was a problem hiding this comment.
The test does not even check it. The string we put into the database consists of the ASCII characters and the test does not assert they are interpreted as unicode.
Actually, it does. The string is encoded in UTF-8 before being passed to the database and MongoDB is indeed known to complain about the encoding sometimes. So, let's keep the test.
|
|
||
| @pytest.mark.tendermint | ||
| def test_get_block_status_for_tx_with_double_inclusion(self, b, alice): | ||
| def test_double_inclusion(self, b, alice): |
There was a problem hiding this comment.
Could you, please, also update the test below (test_text_search)?
We need to replace
# this query only works with MongoDB
try:
assets = list(b.text_search('bigchaindb'))
except OperationError as exc:
assert not isinstance(b.connection, LocalMongoDBConnection)
else:
assert len(assets) == 3
with
assets = list(b.text_search('bigchaindb'))
assert len(assets) == 3
Using LocalMongoDBConnection is part of the test setup, we should not assert for that.
Solution: remove TransactionNotInValidBlock from tests
codegeschrei
force-pushed
the
activate-api-tests
branch
from
88225f2 to
9385bef
Compare
4 participants
Solution: remove or active tests
#2381