fix some errors while running test #1329
Conversation
api/logics.py
Outdated
| order.log( | ||
| f"Suggested mining fee is {order.payout_tx.suggested_mining_fee_rate} Sats/vbyte, the swap fee rate is {order.payout_tx.swap_fee_rate}%" | ||
| ) | ||
| if order.payout_tx is not None: |
There was a problem hiding this comment.
order.payout_tx should not be None. If it is, the order is in a bad/corrupted in some way and the client will receive 500s. This could happen when LND has lower than 300K Sats onchain (node reserve). Are you testing with CLN?
There was a problem hiding this comment.
I am testing with both LND and CLN but without docker. I am writing some scripts that setup an enviornment in regtest that together with roboauto make it quite easy to debug the backed, I will make a PR when I finish it.
This error that the order.payout_tx is None is happening sometimes when I run the tests with CLN, I have now tried a couple of times with LND and it is not happening, but I think that when I initially found it I was running LND. I need to look into it more, it is weird that sometimes it happens and sometimes not, even tough every time I start with a fresh regtest.
There was a problem hiding this comment.
Ok after the cln update this problem seems to have been solved, I have removed this change from the PR.
There was a problem hiding this comment.
Hey @jerryfletcher21, let's give this one a deeper revision.
I have been running the tests without errors so far. So I am wondering what exactly is being fixed here. First lets see whether we are running the tests the same way, just committed a tests readme.md https://github.com/RoboSats/robosats/tree/main/tests#run-e2e-tests
My results and the results of the GH workflow succeed and match:
Results
----------------------------------------------------------------------
Ran 50 tests in 44.572s
OK
Coverage
Name Stmts Miss Cover
-----------------------------------------------------------------------------------------------------
api/__init__.py 0 0 100%
api/admin.py 211 76 64%
api/apps.py 4 0 100%
api/lightning/lnd.py 400 182 54%
api/lightning/node.py 9 4 56%
api/logics.py 961 265 72%
api/management/commands/clean_orders.py 40 15 62%
api/management/commands/follow_invoices.py 123 38 69%
api/migrations/0001_initial.py 10 0 100%
api/migrations/0001_squashed_0036_remove_order_maker_last_seen_and_more.py 13 0 100%
api/migrations/0002_auto_20220307_0821.py 6 0 100%
api/migrations/0003_auto_20220324_2230.py 6 0 100%
api/migrations/0004_alter_currency_currency.py 4 0 100%
api/migrations/0005_alter_order_payment_method.py 4 0 100%
api/migrations/0006_alter_currency_currency.py 4 0 100%
api/migrations/0007_lnpayment_in_flight.py 4 0 100%
api/migrations/0008_auto_20220501_1923.py 7 0 100%
api/migrations/0009_alter_currency_currency.py 4 0 100%
api/migrations/0010_lnpayment_failure_reason.py 4 0 100%
api/migrations/0011_auto_20220527_0057.py 4 0 100%
api/migrations/0012_auto_20220601_2221.py 4 0 100%
api/migrations/0013_auto_20220605_1156.py 5 0 100%
api/migrations/0014_auto_20220619_0535.py 9 0 100%
api/migrations/0015_auto_20220702_1500.py 5 0 100%
api/migrations/0016_alter_onchainpayment_swap_fee_rate.py 4 0 100%
api/migrations/0017_auto_20220710_1127.py 5 0 100%
api/migrations/0018_order_last_satoshis_time.py 4 0 100%
api/migrations/0019_order_contract_finalization_time.py 4 0 100%
api/migrations/0020_auto_20220731_1425.py 5 0 100%
api/migrations/0021_auto_20220813_1333.py 5 0 100%
api/migrations/0022_alter_profile_wants_stealth.py 4 0 100%
api/migrations/0023_alter_currency_currency.py 4 0 100%
api/migrations/0024_auto_20221109_2250.py 5 0 100%
api/migrations/0025_auto_20221127_1135.py 5 0 100%
api/migrations/0026_auto_20230213_2023.py 5 0 100%
api/migrations/0027_auto_20230314_1801.py 4 0 100%
api/migrations/0028_onchainpayment_broadcasted.py 4 0 100%
api/migrations/0029_alter_currency_currency.py 4 0 100%
api/migrations/0030_auto_20230410_1850.py 5 0 100%
api/migrations/0031_auto_20230425_1211.py 5 0 100%
api/migrations/0032_auto_20230430_1419.py 4 0 100%
api/migrations/0033_auto_20230430_1603.py 4 0 100%
api/migrations/0034_auto_20230430_1640.py 4 0 100%
api/migrations/0035_rename_profile_robot.py 5 0 100%
api/migrations/0036_remove_order_maker_last_seen_and_more.py 5 0 100%
api/migrations/0037_lnpayment_order_donated_alter_lnpayment_concept_and_more.py 5 0 100%
api/migrations/0038_alter_lnpayment_order_donated.py 5 0 100%
api/migrations/0039_alter_currency_currency.py 4 0 100%
api/migrations/0040_robot_hash_id.py 4 0 100%
api/migrations/0041_order_logs.py 4 0 100%
api/migrations/0042_alter_order_logs_alter_robot_avatar.py 4 0 100%
api/migrations/0043_order_latitude_order_longitude.py 5 0 100%
api/migrations/0044_alter_markettick_fee_and_more.py 5 0 100%
api/migrations/0045_alter_order_latitude_alter_order_longitude.py 5 0 100%
api/migrations/0046_alter_currency_currency.py 4 0 100%
api/migrations/__init__.py 0 0 100%
api/models/__init__.py 7 0 100%
api/models/currency.py 17 0 100%
api/models/ln_payment.py 65 1 98%
api/models/market_tick.py 31 1 97%
api/models/onchain_payment.py 43 1 98%
api/models/order.py 125 10 92%
api/models/robot.py 51 10 80%
api/nick_generator/dicts/en/adjectives.py 1 0 100%
api/nick_generator/dicts/en/adverbs.py 1 0 100%
api/nick_generator/dicts/en/nouns.py 1 0 100%
api/nick_generator/nick_generator.py 60 8 87%
api/nick_generator/utils.py 6 4 33%
api/notifications.py 140 111 21%
api/oas_schemas.py 30 0 100%
api/serializers.py 167 0 100%
api/tasks.py 160 110 31%
api/tests/__init__.py 0 0 100%
api/tests/test_utils.py 141 4 97%
api/urls.py 5 0 100%
api/utils.py 264 59 78%
api/views.py 494 94 81%
chat/__init__.py 0 0 100%
chat/admin.py 17 0 100%
chat/apps.py 4 0 100%
chat/migrations/0001_initial.py 7 0 100%
chat/migrations/0002_auto_20220528_2255.py 7 0 100%
chat/migrations/__init__.py 0 0 100%
chat/models.py 28 2 93%
chat/serializers.py 33 0 100%
chat/tests.py 0 0 100%
chat/views.py 94 8 91%
control/__init__.py 0 0 100%
control/admin.py 14 0 100%
control/apps.py 4 0 100%
control/migrations/0001_initial.py 5 0 100%
control/migrations/0002_auto_20220619_0535.py 6 0 100%
control/migrations/0003_alter_balancelog_options.py 4 0 100%
control/migrations/__init__.py 0 0 100%
control/models.py 56 1 98%
control/tasks.py 90 12 87%
control/tests.py 0 0 100%
frontend/__init__.py 0 0 100%
frontend/admin.py 0 0 100%
frontend/apps.py 4 0 100%
frontend/tests.py 0 0 100%
frontend/urls.py 3 0 100%
frontend/views.py 8 0 100%
robosats/__init__.py 3 0 100%
robosats/celery/__init__.py 14 0 100%
robosats/celery/conf.py 1 0 100%
robosats/middleware.py 110 34 69%
robosats/settings.py 52 1 98%
robosats/urls.py 9 0 100%
-----------------------------------------------------------------------------------------------------
TOTAL 4362 1051 76%
This is with main 's current commit (#27c07e9).
Can you share your results and the errors you get running the tests before your changes in this PR?
tests/test_trade_pipeline.py
Outdated
| passphrase_path=f"tests/robots/{trade.taker_index}/token", | ||
| private_key_path=f"tests/robots/{trade.taker_index}/enc_priv_key", | ||
| passphrase_path=f"tests/robots/{trade.maker_index}/token", | ||
| private_key_path=f"tests/robots/{trade.maker_index}/enc_priv_key", |
There was a problem hiding this comment.
Well, technically both robots could have a reward to claim. But the robot that cancelled unilaterally in this test is the maker robot (trade.cancel_order(trade.maker_index)) and the signed payout invoice is submitted by the taker two lines below self.client.post(path, body, **taker_headers) so this change here should not work 😵 😄
There was a problem hiding this comment.
To me it is happening with both LND and CLN every time. When I first looked into this error, I changed from taker to maker there without really looking at the test, and when it succeded I moved on, my bad :)
Now looking at it again I think the error is at line 1003 taker_headers = trade.get_robot_auth(trade.maker_index), it should be trade.taker_index, with this I get no errors. Why was it not failing to you though?
There was a problem hiding this comment.
I will try to research this one once I get a bit og time
| signed_message = gpg.sign( | ||
| message, passphrase=passphrase, extra_args=["--digest-algo", "SHA512"] | ||
| message, keyid=import_result.fingerprints[0], passphrase=passphrase, |
There was a problem hiding this comment.
Makes sense, not sure why it was working (or not) before 😅
There was a problem hiding this comment.
Without this to me every tests that uses sign_message fails, also with roboauto I remember that it took me some failed attempts before I found that I had to also put keyid. Are the tests not failing to you?
There was a problem hiding this comment.
Ok I read now that it is not failing to you, strange I do not know why I have to put keyid
There was a problem hiding this comment.
No tests pass both locally and on the GH workflow "as is"
jerryfletcher21
force-pushed
the
fix-tests
branch
from
3515210 to
a8e3f67
Compare
jerryfletcher21
force-pushed
the
fix-tests
branch
from
a8e3f67 to
7bfafc9
Compare
|
@Reckless-Satoshi ok now everything is clear to me here. |
Reckless-Satoshi
force-pushed
the
main
branch
2 times, most recently
from
9054b87 to
f9244e1
Compare
What does this PR do?
This PR fixes some errors that occured while running
python3 manage.py test.Checklist before merging
pip install pre-commit, thenpre-commit install. Pre-commit installs git hooks that automatically check the codebase. If pre-commit fails when you commit your changes, please fix the problems it points out.