Add basic Qt wallet test #9974

ryanofsky · 2017-03-10T21:56:47Z

This adds a basic unit test for the Qt send coins dialog. I have more changes (for a future PR) that extend this to cover new Qt bumpfee code in #9697.

Previously, I brought up the topic of qt unit tests in #9817 (comment) and #9697 (comment).

Move Boost.Test main function and global overrides to a new test_bitcoin_main.cpp file.

Reset global state after rpc tests, and remove unnecessary ECC initialization to prevent assert error if it is initialized twice.

luke-jr · 2017-03-11T01:34:06Z

I almost wonder if it might be useful to have a RPC wrapper for the GUI (possibly in addition to this) so we can just use the RPC tests with it as well.

fanquake · 2017-03-11T02:00:24Z

Failed with:

==============================================
   Bitcoin Core 0.14.99: src/test-suite.log
==============================================
# TOTAL: 2
# PASS:  1
# SKIP:  0
# XFAIL: 0
# FAIL:  1
# XPASS: 0
# ERROR: 0
.. contents:: :depth: 2
FAIL: qt/test/test_bitcoin-qt
=============================
This application failed to start because it could not find or load the Qt platform plugin "xcb"
in "".
Reinstalling the application may fix this problem.

laanwj · 2017-03-11T08:39:29Z

Concept ACK

maflcko · 2017-03-11T09:31:11Z

Concept ACK for sure, as we basically don't have any gui tests right now. In the long term I'd prefer a solution that runs on a higher abstraction level. Your current approach is pretty much mimicking functional tests on a low (unit test) level.

ryanofsky · 2017-03-11T09:51:03Z

I'd prefer a solution that runs on a higher abstraction level. Your current approach is pretty much mimicking functional tests on a low (unit test) level.

I don't think I understand. I'm not doing anything original here, just following the approach of http://doc.qt.io/qt-5/qttestlib-tutorial3-example.html.

Is it that instead of the writing a test that sends qt events, you would prefer to see a test that starts an x server and sends keyboard and mouse events?

jonasschnelli · 2017-03-11T11:36:11Z

I really like this.
Concept ACK.

maflcko · 2017-03-11T11:54:28Z

Is it that instead of the writing a test that sends qt events, you would

prefer to see a test that starts an x server and sends keyboard and mouse events? Indeed. I was dreaming of a gui test framework where you just press a "record this" button. Then records discrete actions (e.g. "Press: Receive tab", "Press: Copy address", other gui: "Press: Send tab", "Insert: $address", ...) instead of the exact mouse and keyboard movements, such that it works on different screen resolutions. Though, we also need a way to wait for the gui to catch up on longer tasks such as importmulti with rescan, so I don't think this is a weekend project.

…

On Sat, Mar 11, 2017 at 12:36 PM, Jonas Schnelli ***@***.***> wrote: I really like this. Concept ACK. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#9974 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AGGmv2gRizL47AbknwIa11DdwdLFQFBrks5rkoc4gaJpZM4MZ6Pd> .

fanquake · 2017-03-12T03:24:37Z

src/qt/test/test_main.cpp

    bool fInvalid = false;

    // Don't remove this, it's needed to access
    // QCoreApplication:: in the tests


Comment needs removing/updating?

It's still accurate, but I expanded the comment in f4d28fb.

fanquake · 2017-03-12T03:25:23Z

Still failing Travis with the same issue, @theuni should probably take a look here.

maflcko · 2017-03-12T11:40:16Z

src/qt/test/test_main.cpp

        fInvalid = true;
+#ifdef ENABLE_WALLET
+    WalletTests test5;
+    if (QTest::qExec(&test5) != 0)


style nit: We use braces now; Always.

Added in b61b34c

Avoids following error when qt is statically linked into the test binary, as on travis: This application failed to start because it could not find or load the Qt platform plugin "xcb" in "".

Avoids following error: QXcbConnection: Could not connect to display

ryanofsky

Finally got test to pass on travis.

ryanofsky · 2017-03-15T17:25:42Z

src/qt/test/test_main.cpp

    bool fInvalid = false;

    // Don't remove this, it's needed to access
    // QCoreApplication:: in the tests


It's still accurate, but I expanded the comment in f4d28fb.

ryanofsky · 2017-03-15T17:26:06Z

src/qt/test/test_main.cpp

        fInvalid = true;
+#ifdef ENABLE_WALLET
+    WalletTests test5;
+    if (QTest::qExec(&test5) != 0)


Added in b61b34c

jonasschnelli · 2017-03-16T13:31:39Z

A lock is missing.

diff --git a/src/qt/test/wallettests.cpp b/src/qt/test/wallettests.cpp
index 81b358e..d63f4ec 100644
--- a/src/qt/test/wallettests.cpp
+++ b/src/qt/test/wallettests.cpp
@@ -73,6 +73,7 @@ void WalletTests::walletTests()
     test.CreateAndProcessBlock({}, GetScriptForRawPubKey(test.coinbaseKey.GetPubKey()));
     bitdb.MakeMock();
     CWallet wallet("wallet_test.dat");
+    LOCK(wallet.cs_wallet);
     bool firstRun;
     wallet.LoadWallet(firstRun);
     wallet.SetAddressBook(test.coinbaseKey.GetPubKey().GetID(), "", "receive");

How could this pass travis with the missing lock?

Will properly ACK after apply the diff above.

Question: is it possible to create a screenshot (or a bitmap/JPG/PNG) of the UI elements you open?
A such form of an UI export would be great. We could add this then to a CI and have plenty of screenshots from various operating systems in order to verify the UI correctness.

ryanofsky · 2017-03-16T13:52:04Z

A lock is missing.

Will add. I'm not sure the way the lock is acquired in the diff is exactly right, because cs_main is supported to be acquired before cs_wallet, and some wallet methods acquire both.

How could this pass travis with the missing lock?

Locally, the test works fine for me with the missing lock. On travis, every configuration we have either specifies NO_QT or NO_WALLET, or omits RUN_TESTS, so neither PaymentServerTests code nor this new WalletTests code currently executes.

Question: is it possible to create a screenshot (or a bitmap/JPG/PNG) of the UI elements you open?
A such form of an UI export would be great. We could add this then to a CI and have plenty of screenshots from various operating systems in order to verify the UI correctness.

Something like this should be possible. To help debug this change, I frequently added widget.show(); QEventLoop().exec(); calls to be able to see and interact with individual test widgets, so it should be possible to capture images also (maybe with QWidget::grab?)

Fix missing lock pointed out bitcoin#9974 (comment)

ryanofsky · 2017-03-16T18:35:42Z

Fixed locking bug in 739f19c

Squashed 739f19c -> 9576b01 (pr/qt-test.7 -> pr/qt-test.8)

maflcko · 2017-03-17T00:17:19Z

utACK 9576b01

jonasschnelli · 2017-03-17T13:27:25Z

Tested ACK 9576b01

634e38c [Tests] Add Qt GUI tests to Overview and ReceiveCoin Page (Anditto Heristyo) Pull request description: I've added some Qt wallet tests based on #9974, namely the input & buttons on ReceiveCoin. Tree-SHA512: f4223827145e35c2abee83a6ca777498bebcff3825fece10fbb1dbfd1f6bb017d3f2c0521662854b4407cdeee9c6a527269ab9cc28e0dc85c11b668155fcd195

Construct scoped_connection directly instead of relying on copy initialization and move constructor. Avoids the following compile error in debian jessie: ``` In file included from /usr/include/boost/signals2/signal.hpp:21:0, from ./util.h:29, from ./dbwrapper.h:11, from ./txdb.h:10, from ./test/test_bitcoin.h:11, from qt/test/wallettests.cpp:11: /usr/include/boost/signals2/connection.hpp: In function ‘uint256 {anonymous}::SendCoins(CWallet&, SendCoinsDialog&, const CBitcoinAddress&, CAmount)’: /usr/include/boost/signals2/connection.hpp:234:7: error: ‘boost::signals2::scoped_connection::scoped_connection(const boost::signals2::scoped_connection&)’ is private scoped_connection(const scoped_connection &other); ^ qt/test/wallettests.cpp:47:6: error: within this context }); ^ ``` Error reported by Pavel Janík <Pavel@Janik.cz> in bitcoin#9974 (comment) Conflicts: src/qt/test/wallettests.cpp

Construct QTimer object directly, instead of relying on QTimer::singleShot overloads accepting lambdas, which weren't introduced until Qt 5.4. Avoids the following compile error in debian jessie: ``` qt/test/wallettests.cpp: In function ‘void {anonymous}::ConfirmSend()’: qt/test/wallettests.cpp:34:6: error: no matching function for call to ‘QTimer::singleShot(int, Qt::TimerType, {anonymous}::ConfirmSend()::<lambda()>)’ }); ^ qt/test/wallettests.cpp:34:6: note: candidates are: In file included from /usr/include/x86_64-linux-gnu/qt5/QtCore/QTimer:1:0, from ./qt/sendcoinsdialog.h:13, from qt/test/wallettests.cpp:7: /usr/include/x86_64-linux-gnu/qt5/QtCore/qtimer.h:81:17: note: static void QTimer::singleShot(int, const QObject*, const char*) static void singleShot(int msec, const QObject *receiver, const char *member); ^ /usr/include/x86_64-linux-gnu/qt5/QtCore/qtimer.h:81:17: note: no known conversion for argument 2 from ‘Qt::TimerType’ to ‘const QObject*’ /usr/include/x86_64-linux-gnu/qt5/QtCore/qtimer.h:82:17: note: static void QTimer::singleShot(int, Qt::TimerType, const QObject*, const char*) static void singleShot(int msec, Qt::TimerType timerType, const QObject *receiver, const char *member); ^ /usr/include/x86_64-linux-gnu/qt5/QtCore/qtimer.h:82:17: note: candidate expects 4 arguments, 3 provided ``` Error reported by Pavel Janík <Pavel@Janik.cz> in bitcoin#9974 (comment)

9576b01 Enable xvfb in travis to allow running test_bitcoin-qt (Russell Yanofsky) 9e6817e Add new test_bitcoin-qt static library dependencies (Russell Yanofsky) 2754ef1 Add simple qt wallet test sending a transaction (Russell Yanofsky) b61b34c Add braces to if statements in Qt test_main (Russell Yanofsky) cc9503c Make qt test compatible with TestChain100Setup framework (Russell Yanofsky) 91e3035 Make test_bitcoin.cpp compatible with Qt Test framework (Russell Yanofsky) Tree-SHA512: da491181848b8c39138e997ae5ff2df0b16eef2d9cdd0a965229b1a28d4fa862d5f1ef314a1736e5050e88858f329124d15c689659fc6e50fefde769ba24e523

9576b01 Enable xvfb in travis to allow running test_bitcoin-qt (Russell Yanofsky) 9e6817e Add new test_bitcoin-qt static library dependencies (Russell Yanofsky) 2754ef1 Add simple qt wallet test sending a transaction (Russell Yanofsky) b61b34c Add braces to if statements in Qt test_main (Russell Yanofsky) cc9503c Make qt test compatible with TestChain100Setup framework (Russell Yanofsky) 91e3035 Make test_bitcoin.cpp compatible with Qt Test framework (Russell Yanofsky) Tree-SHA512: da491181848b8c39138e997ae5ff2df0b16eef2d9cdd0a965229b1a28d4fa862d5f1ef314a1736e5050e88858f329124d15c689659fc6e50fefde769ba24e523 remove line, testing bitcoin -> dash, testing bitcoin -> dash, testing resolve name conflict, testing bitcoin -> dash re-add test fixture line set dash pointers to nullptr after deletion move ExceptionInitializer into test_dash_main.cpp code review, fix tests Signed-off-by: Pasta <Pasta@dash.org>

9576b01 Enable xvfb in travis to allow running test_bitcoin-qt (Russell Yanofsky) 9e6817e Add new test_bitcoin-qt static library dependencies (Russell Yanofsky) 2754ef1 Add simple qt wallet test sending a transaction (Russell Yanofsky) b61b34c Add braces to if statements in Qt test_main (Russell Yanofsky) cc9503c Make qt test compatible with TestChain100Setup framework (Russell Yanofsky) 91e3035 Make test_bitcoin.cpp compatible with Qt Test framework (Russell Yanofsky) Tree-SHA512: da491181848b8c39138e997ae5ff2df0b16eef2d9cdd0a965229b1a28d4fa862d5f1ef314a1736e5050e88858f329124d15c689659fc6e50fefde769ba24e523 remove line, testing bitcoin -> dash, testing bitcoin -> dash, testing resolve name conflict, testing bitcoin -> dash re-add test fixture line code review, fix tests Signed-off-by: Pasta <Pasta@dash.org> move ExceptionInitializer into test_dash_main.cpp remove witness from nulldummy.py Signed-off-by: Pasta <Pasta@dash.org> change error text to match expected Signed-off-by: Pasta <Pasta@dash.org>

…Coin Page 634e38c [Tests] Add Qt GUI tests to Overview and ReceiveCoin Page (Anditto Heristyo) Pull request description: I've added some Qt wallet tests based on bitcoin#9974, namely the input & buttons on ReceiveCoin. Tree-SHA512: f4223827145e35c2abee83a6ca777498bebcff3825fece10fbb1dbfd1f6bb017d3f2c0521662854b4407cdeee9c6a527269ab9cc28e0dc85c11b668155fcd195

9576b01 Enable xvfb in travis to allow running test_bitcoin-qt (Russell Yanofsky) 9e6817e Add new test_bitcoin-qt static library dependencies (Russell Yanofsky) 2754ef1 Add simple qt wallet test sending a transaction (Russell Yanofsky) b61b34c Add braces to if statements in Qt test_main (Russell Yanofsky) cc9503c Make qt test compatible with TestChain100Setup framework (Russell Yanofsky) 91e3035 Make test_bitcoin.cpp compatible with Qt Test framework (Russell Yanofsky) Tree-SHA512: da491181848b8c39138e997ae5ff2df0b16eef2d9cdd0a965229b1a28d4fa862d5f1ef314a1736e5050e88858f329124d15c689659fc6e50fefde769ba24e523 remove line, testing bitcoin -> dash, testing bitcoin -> dash, testing resolve name conflict, testing bitcoin -> dash re-add test fixture line code review, fix tests Signed-off-by: Pasta <Pasta@dash.org> move ExceptionInitializer into test_dash_main.cpp remove witness from nulldummy.py Signed-off-by: Pasta <Pasta@dash.org> change error text to match expected Signed-off-by: Pasta <Pasta@dash.org>

…Coin Page 634e38c [Tests] Add Qt GUI tests to Overview and ReceiveCoin Page (Anditto Heristyo) Pull request description: I've added some Qt wallet tests based on bitcoin#9974, namely the input & buttons on ReceiveCoin. Tree-SHA512: f4223827145e35c2abee83a6ca777498bebcff3825fece10fbb1dbfd1f6bb017d3f2c0521662854b4407cdeee9c6a527269ab9cc28e0dc85c11b668155fcd195

ryanofsky added 2 commits March 10, 2017 15:47

Make test_bitcoin.cpp compatible with Qt Test framework

91e3035

Move Boost.Test main function and global overrides to a new test_bitcoin_main.cpp file.

Make qt test compatible with TestChain100Setup framework

cc9503c

Reset global state after rpc tests, and remove unnecessary ECC initialization to prevent assert error if it is initialized twice.

fanquake added the Tests label Mar 10, 2017

maflcko added the GUI label Mar 10, 2017

fanquake reviewed Mar 12, 2017

View reviewed changes

maflcko reviewed Mar 12, 2017

View reviewed changes

ryanofsky force-pushed the pr/qt-test branch from 3feca35 to f8be60b Compare March 15, 2017 00:54

ryanofsky added 4 commits March 15, 2017 13:02

Add braces to if statements in Qt test_main

b61b34c

Add simple qt wallet test sending a transaction

2754ef1

Add new test_bitcoin-qt static library dependencies

9e6817e

Avoids following error when qt is statically linked into the test binary, as on travis: This application failed to start because it could not find or load the Qt platform plugin "xcb" in "".

Enable xvfb in travis to allow running test_bitcoin-qt

9576b01

Avoids following error: QXcbConnection: Could not connect to display

ryanofsky force-pushed the pr/qt-test branch 3 times, most recently from f685681 to 956e61e Compare March 15, 2017 17:26

ryanofsky commented Mar 15, 2017

View reviewed changes

ryanofsky added a commit to ryanofsky/bitcoin that referenced this pull request Mar 16, 2017

fixup! Add simple qt wallet test sending a transaction

739f19c

Fix missing lock pointed out bitcoin#9974 (comment)

ryanofsky force-pushed the pr/qt-test branch from 739f19c to 9576b01 Compare March 16, 2017 18:33

anditto mentioned this pull request Sep 19, 2017

[Tests] Add Qt GUI tests to Overview and ReceiveCoin Page #11365

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Add basic Qt wallet test #9974

Add basic Qt wallet test #9974

Uh oh!

Conversation

ryanofsky commented Mar 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

luke-jr commented Mar 11, 2017

Uh oh!

fanquake commented Mar 11, 2017

Uh oh!

laanwj commented Mar 11, 2017

Uh oh!

maflcko commented Mar 11, 2017

Uh oh!

ryanofsky commented Mar 11, 2017

Uh oh!

jonasschnelli commented Mar 11, 2017

Uh oh!

maflcko commented Mar 11, 2017 via email

Uh oh!

fanquake Mar 12, 2017

Choose a reason for hiding this comment

Uh oh!

ryanofsky Mar 15, 2017

Choose a reason for hiding this comment

Uh oh!

fanquake commented Mar 12, 2017

Uh oh!

maflcko Mar 12, 2017

Choose a reason for hiding this comment

Uh oh!

ryanofsky Mar 15, 2017

Choose a reason for hiding this comment

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

ryanofsky Mar 15, 2017

Choose a reason for hiding this comment

Uh oh!

ryanofsky Mar 15, 2017

Choose a reason for hiding this comment

Uh oh!

jonasschnelli commented Mar 16, 2017

Uh oh!

ryanofsky commented Mar 16, 2017

Uh oh!

ryanofsky commented Mar 16, 2017

Uh oh!

maflcko commented Mar 17, 2017

Uh oh!

jonasschnelli commented Mar 17, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

ryanofsky commented Mar 10, 2017 •

edited

Loading