search

nanocurrency / nano-node

Nano is digital currency. Its ticker is: XNO and its currency symbol is: Ӿ
https://nano.org
BSD 3-Clause "New" or "Revised" License
3.48k stars 787 forks source link

Failing test case: qt_test wallet.status #3235

Open dsiganos opened 3 years ago

dsiganos commented 3 years ago

The test is failing because it is waiting for a wallet to attain the synchronising state. Synchronising state is when a bootstrap attempt is in progress.

A boostrap attempt starts but it is abandoned immediately due to "pulling" being false. The failure is caused by the commit: https://github.com/nanocurrency/nano-node/commit/a3f15515431769467b9c7be1dbf7d0163cbdad64 a3f15515 ("Combine alarm and worker threads into a pool (#2871)", 2021-02-01)

In software before that commit, the synchronising state stays in the wallet's active status just about long enough to be noticed by the test function. That's due to more context switches happening due to the use of the alarm class and Qt processEvents not processing both the attempt started and attempt ended in one call.

After that commit, the processing of events is more efficient and the synchronising is added and removed in one call of processEvents and the test function does not see the synchronising state spike.

But, having said all that, I believe the commit above is not the problem but a trigger for the problem that had already existed. It seems to me that the test case is wrong and needs fixing.

dsiganos commented 3 years ago

Possible solution. Calling it a possible solution because I do not think it is a good solution. https://github.com/dsiganos/nano-node/commit/f7f1134d8b9cfce8a62eb5268212f9a48f3a89cd It seems wrong to bypass the status check and add an observer directly into the node.