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

functional_tests: fix rpc payment tests spurious failure and speedup signature generation #6309

Merged
merged 2 commits into from Apr 4, 2020

Conversation

@moneromooo-monero
Copy link
Collaborator

moneromooo-monero commented Jan 27, 2020

No description provided.

@vtnerd
vtnerd approved these changes Mar 4, 2020
uint32_t count = 1;
if (argc == 3)
{
count = atoi(argv[2]);

This comment has been minimized.

Copy link
@vtnerd

vtnerd Mar 4, 2020

Contributor

atoi returns int

This comment has been minimized.

Copy link
@moneromooo-monero

moneromooo-monero Mar 7, 2020

Author Collaborator

By this, do you mean "check for negative values" ? It's a test program so there's not much point. Or did you mean "use an int, not uint32_t" ?

@moneromooo-monero moneromooo-monero force-pushed the moneromooo-monero:fts branch from bccb7e2 to 6cbf102 Mar 20, 2020
@selsta

This comment has been minimized.

Copy link
Contributor

selsta commented Mar 20, 2020

Functional tests CI failed.

2020-03-20T17:48:59.1366737Z Testing access mining
2020-03-20T17:48:59.1366847Z Traceback (most recent call last):
2020-03-20T17:48:59.1366986Z   File "/home/runner/work/monero/monero/tests/functional_tests/rpc_payment.py", line 449, in <module>
2020-03-20T17:48:59.1367123Z     RPCPaymentTest().run_test()
2020-03-20T17:48:59.1367266Z   File "/home/runner/work/monero/monero/tests/functional_tests/rpc_payment.py", line 50, in run_test
2020-03-20T17:48:59.1367401Z     self.test_access_mining()
2020-03-20T17:48:59.1367542Z   File "/home/runner/work/monero/monero/tests/functional_tests/rpc_payment.py", line 295, in test_access_mining
2020-03-20T17:48:59.1368442Z     assert nonce < 1000 # can't find both valid and invalid -> the RPC probably fails
2020-03-20T17:48:59.1368580Z AssertionError
2020-03-20T17:48:59.1368688Z [TEST FAILED] rpc_payment
@moneromooo-monero

This comment has been minimized.

Copy link
Collaborator Author

moneromooo-monero commented Mar 22, 2020

Works for me.

@selsta

This comment has been minimized.

Copy link
Contributor

selsta commented Mar 23, 2020

Also works for me locally but not in CI. CI does work consistently without this PR. Maybe someone else has an idea.

@moneromooo-monero

This comment has been minimized.

Copy link
Collaborator Author

moneromooo-monero commented Mar 23, 2020

Do you have daemon logs for this when it fails ?

@selsta

This comment has been minimized.

Copy link
Contributor

selsta commented Mar 24, 2020

This is the full log when running make release-test. Are you able to find the failing lines?

bitmonero.log

@moneromooo-monero

This comment has been minimized.

Copy link
Collaborator Author

moneromooo-monero commented Mar 24, 2020

No apparent error. Wallet logs maybe ?

@xiphon

This comment has been minimized.

Copy link
Contributor

xiphon commented Mar 24, 2020

Think the reason is inconsistent STALE_THRESHOLD, it is 10s in rpc_payment test and 15s in the daemon code (rpc_payment.cpp).

diff --git "a/tests/functional_tests/rpc_payment.py" "b/tests/functional_tests/rpc_payment.py"
index 9a968d073..4d06feeb1 100644
--- "a/tests/functional_tests/rpc_payment.py"
+++ "b/tests/functional_tests/rpc_payment.py"
@@ -45,6 +45,7 @@ class RPCPaymentTest():
         assert len(self.make_test_signature) > 0
         self.secret_key, self.public_key = self.get_keys()
         self.signatures = []
+        self.stale_threshold = 15
         self.reset()
         self.test_access_tracking()
         self.test_access_mining()
@@ -170,7 +171,7 @@ class RPCPaymentTest():
             assert nonce < 1000 # can't find both valid and invalid -> the RPC probably fails
             last_credits = res.credits
 
-            if time.time() >= loop_time + 10:
+            if time.time() >= loop_time + self.stale_threshold:
                 res = daemon.rpc_access_info(client = self.get_signature())
                 cookie = res.cookie
                 loop_time = time.time()
@@ -206,7 +207,7 @@ class RPCPaymentTest():
             except:
                 found_invalid += 1
             assert nonce < 1000 # can't find a valid none -> the RPC probably fails
-            if time.time() >= loop_time + 10:
+            if time.time() >= loop_time + self.stale_threshold:
                 res = daemon.rpc_access_info(client = self.get_signature())
                 cookie = res.cookie
                 loop_time = time.time()
@@ -243,7 +244,7 @@ class RPCPaymentTest():
                 found_invalid += 1
             assert nonce < 1000 # can't find both valid and invalid -> the RPC probably fails
 
-            if time.time() >= loop_time + 10:
+            if time.time() >= loop_time + self.stale_threshold:
                 res = daemon.rpc_access_info(client = self.get_signature())
                 cookie = res.cookie
                 loop_time = time.time()
@@ -294,7 +295,7 @@ class RPCPaymentTest():
                     found_invalid += 1
             assert nonce < 1000 # can't find both valid and invalid -> the RPC probably fails
 
-            if time.time() >= loop_time + 10:
+            if time.time() >= loop_time + self.stale_threshold:
                 res = daemon.rpc_access_info(client = self.get_signature())
                 # cookie = res.cookie # let the daemon update its timestamp, but use old cookie
                 loop_time = time.time()
@selsta

This comment has been minimized.

Copy link
Contributor

selsta commented Mar 24, 2020

@moneromooo-monero

This comment has been minimized.

Copy link
Collaborator Author

moneromooo-monero commented Mar 24, 2020

It's intended. The daemon rejects after 15 seconds, so the test refreshes after 10 to be sure not to be stale. If this actually fixes it, it would be nice to know why.

@selsta

This comment has been minimized.

Copy link
Contributor

selsta commented Mar 24, 2020

@moneromooo-monero these should be all the logs

functional_tests.zip
rpc-log.zip

@moneromooo-monero

This comment has been minimized.

Copy link
Collaborator Author

moneromooo-monero commented Mar 25, 2020

Does this help ?

diff --git a/tests/functional_tests/rpc_payment.py b/tests/functional_tests/rpc_payment.py
index 9a968d073..b15d7c3ca 100755
--- a/tests/functional_tests/rpc_payment.py
+++ b/tests/functional_tests/rpc_payment.py
@@ -280,6 +280,7 @@ class RPCPaymentTest():
         found_close_stale = 0
         found_late_stale = 0
         loop_time = time.time()
+        time.sleep(15)
         while found_close_stale == 0 or found_late_stale == 0:
             nonce += 1
             try:
Otherwise the daemon will start rejecting
Executing a new binary for each signature can get really slow
@moneromooo-monero moneromooo-monero force-pushed the moneromooo-monero:fts branch from 9444f3f to f5a11f0 Mar 26, 2020
@selsta
selsta approved these changes Mar 26, 2020
@luigi1111 luigi1111 merged commit 0bd2c14 into monero-project:master Apr 4, 2020
5 checks passed
5 checks passed
build-macos
Details
build-windows
Details
build-ubuntu
Details
libwallet-ubuntu
Details
test-ubuntu
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

5 participants
You can’t perform that action at this time.