Add BitcoinApplication & RPCConsole tests #11625

ryanofsky · 2017-11-07T01:19:03Z

Add test coverage for Qt initialization code & basic RPC console functionality

Motivation for this change was a bug in #11603 which existing tests failed to catch.

fanquake · 2017-11-07T01:49:40Z

Travis failures:

In file included from /usr/include/qt4/QtGui/QLineEdit:1:0,
                 from qt/test/apptests.cpp:20:
/usr/include/qt4/QtGui/qlineedit.h: In function ‘void {anonymous}::TestRpcCommand(RPCConsole*)’:
/usr/include/qt4/QtGui/qlineedit.h:198:10: error: ‘void QLineEdit::returnPressed()’ is protected
     void returnPressed();
          ^
qt/test/apptests.cpp:34:29: error: within this context
     lineEdit->returnPressed();
                             ^
make[2]: *** [qt/test/qt_test_test_bitcoin_qt-apptests.o] Error 1

FAIL: qt/test/test_bitcoin-qt
=============================
********* Start testing of AppTests *********
Config: Using QtTest library 5.7.1, Qt 5.7.1 (x86_64-little_endian-lp64 static release build; by GCC 4.8.4)
PASS   : AppTests::initTestCase()
QWARN  : AppTests::appTests() Backing up GUI settings to "/tmp/test_bitcoin-qt_1510018491_71945/regtest/guisettings.ini.bak"
QWARN  : AppTests::appTests() QFont::setPointSizeF: Point size <= 0 (-1.000000), must be greater than 0
QWARN  : AppTests::appTests() QFont::setPointSizeF: Point size <= 0 (-1.000000), must be greater than 0
QWARN  : AppTests::appTests() QFont::setPointSizeF: Point size <= 0 (-1.000000), must be greater than 0
QWARN  : AppTests::appTests() QFont::setPointSizeF: Point size <= 0 (-1.000000), must be greater than 0
QWARN  : AppTests::appTests() QFont::setPointSizeF: Point size <= 0 (-1.000000), must be greater than 0
QWARN  : AppTests::appTests() QFont::setPointSizeF: Point size <= 0 (-1.000000), must be greater than 0
QWARN  : AppTests::appTests() QFont::setPointSizeF: Point size <= 0 (-1.000000), must be greater than 0
QWARN  : AppTests::appTests() QFont::setPointSizeF: Point size <= 0 (-1.000000), must be greater than 0
QDEBUG : AppTests::appTests() requestInitialize : Requesting initialize
QDEBUG : AppTests::appTests() initialize : Running initialization in thread
QDEBUG : AppTests::appTests() initializeResult : Initialization result:  true
QWARN  : AppTests::appTests() Platform customization: "other"
========= Received signal, dumping stack ==============
========= End of stack trace ==============
QFATAL : AppTests::appTests() Received signal 11
         Function time: 267ms Total time: 272ms
FAIL!  : AppTests::appTests() Received a fatal error.
   Loc: [Unknown file(0)]
Totals: 1 passed, 1 failed, 0 skipped, 0 blacklisted, 284ms
********* Finished testing of AppTests *********

promag

LGTM. Some nits below.

src/qt/test/apptests.cpp

src/qt/test/apptests.h

ryanofsky

Had to make some fixes for travis: 5be833c -> 4bf5ecd (pr/apptest.1 -> pr/apptest.3), there was QSystemTrayIcon segfault happening with an older version of qt5 and there were compile errors with qt4.

src/qt/test/apptests.cpp

src/qt/test/apptests.h

src/qt/test/apptests.cpp

ryanofsky

Had to make some more #include fixes, but travis is actually fixed now. Updated 4bf5ecd -> e94e344 (pr/apptest.3 -> pr/apptest.4)

src/qt/test/apptests.cpp

promag

Some nits otherwise LGTM. Will test locally.

src/qt/bitcoingui.cpp

src/qt/test/apptests.cpp

promag · 2017-11-09T01:16:19Z

Any hint?

make clean && make

...

  CXXLD    qt/test/test_bitcoin-qt
Undefined symbols for architecture x86_64:
  "MacNotificationHandler::instance()", referenced from:
      Notificator::Notificator(QString const&, QSystemTrayIcon*, QWidget*) in libbitcoinqt.a(qt_libbitcoinqt_a-notificator.o)
      Notificator::notify(Notificator::Class, QString const&, QString const&, QIcon const&, int) in libbitcoinqt.a(qt_libbitcoinqt_a-notificator.o)
  "MacNotificationHandler::hasUserNotificationCenterSupport()", referenced from:
      Notificator::Notificator(QString const&, QSystemTrayIcon*, QWidget*) in libbitcoinqt.a(qt_libbitcoinqt_a-notificator.o)
  "MacNotificationHandler::showNotification(QString const&, QString const&)", referenced from:
      Notificator::notify(Notificator::Class, QString const&, QString const&, QIcon const&, int) in libbitcoinqt.a(qt_libbitcoinqt_a-notificator.o)
  "MacDockIconHandler::cleanup()", referenced from:
      BitcoinGUI::~BitcoinGUI() in libbitcoinqt.a(qt_libbitcoinqt_a-bitcoingui.o)
  "MacDockIconHandler::setMainWindow(QMainWindow*)", referenced from:
      BitcoinGUI::createTrayIconMenu() in libbitcoinqt.a(qt_libbitcoinqt_a-bitcoingui.o)
  "MacDockIconHandler::dockMenu()", referenced from:
      BitcoinGUI::createTrayIconMenu() in libbitcoinqt.a(qt_libbitcoinqt_a-bitcoingui.o)
  "MacDockIconHandler::instance()", referenced from:
      BitcoinGUI::BitcoinGUI(PlatformStyle const*, NetworkStyle const*, QWidget*) in libbitcoinqt.a(qt_libbitcoinqt_a-bitcoingui.o)
      BitcoinGUI::createTrayIconMenu() in libbitcoinqt.a(qt_libbitcoinqt_a-bitcoingui.o)
  "MacDockIconHandler::setIcon(QIcon const&)", referenced from:
      BitcoinGUI::BitcoinGUI(PlatformStyle const*, NetworkStyle const*, QWidget*) in libbitcoinqt.a(qt_libbitcoinqt_a-bitcoingui.o)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [qt/test/test_bitcoin-qt] Error 1
make[2]: *** Waiting for unfinished jobs....
make[1]: *** [all-recursive] Error 1
make: *** [all-recursive] Error 1

jonasschnelli · 2017-11-29T23:40:41Z

Have the same compile issue (clang):

Undefined symbols for architecture x86_64:
  "MacNotificationHandler::instance()", referenced from:
      Notificator::Notificator(QString const&, QSystemTrayIcon*, QWidget*) in libbitcoinqt.a(qt_libbitcoinqt_a-notificator.o)
      Notificator::notifyMacUserNotificationCenter(Notificator::Class, QString const&, QString const&, QIcon const&) in libbitcoinqt.a(qt_libbitcoinqt_a-notificator.o)
  "MacNotificationHandler::hasUserNotificationCenterSupport()", referenced from:
      Notificator::Notificator(QString const&, QSystemTrayIcon*, QWidget*) in libbitcoinqt.a(qt_libbitcoinqt_a-notificator.o)
  "MacNotificationHandler::showNotification(QString const&, QString const&)", referenced from:
      Notificator::notifyMacUserNotificationCenter(Notificator::Class, QString const&, QString const&, QIcon const&) in libbitcoinqt.a(qt_libbitcoinqt_a-notificator.o)
  "MacDockIconHandler::cleanup()", referenced from:
      BitcoinGUI::~BitcoinGUI() in libbitcoinqt.a(qt_libbitcoinqt_a-bitcoingui.o)
  "MacDockIconHandler::setMainWindow(QMainWindow*)", referenced from:
      BitcoinGUI::createTrayIconMenu() in libbitcoinqt.a(qt_libbitcoinqt_a-bitcoingui.o)
  "MacDockIconHandler::dockMenu()", referenced from:
      BitcoinGUI::createTrayIconMenu() in libbitcoinqt.a(qt_libbitcoinqt_a-bitcoingui.o)
  "MacDockIconHandler::instance()", referenced from:
      BitcoinGUI::BitcoinGUI(PlatformStyle const*, NetworkStyle const*, QWidget*) in libbitcoinqt.a(qt_libbitcoinqt_a-bitcoingui.o)
      BitcoinGUI::createTrayIconMenu() in libbitcoinqt.a(qt_libbitcoinqt_a-bitcoingui.o)
  "MacDockIconHandler::setIcon(QIcon const&)", referenced from:
      BitcoinGUI::BitcoinGUI(PlatformStyle const*, NetworkStyle const*, QWidget*) in libbitcoinqt.a(qt_libbitcoinqt_a-bitcoingui.o)
ld: symbol(s) not found for architecture x86_64
clang: error: linker command failed with exit code 1 (use -v to see invocation)
make[2]: *** [qt/test/test_bitcoin-qt] Error 1
make[1]: *** [all-recursive] Error 1

Looks like an OSX issue.

ryanofsky · 2017-11-30T20:53:03Z

Looking into the link error, but @jonasschnelli do you have an idea why the build might be failing for you even though the apple target succeeds on travis?

ryanofsky · 2017-11-30T21:08:18Z

Looking into the link error, but @jonasschnelli do you have an idea why the build might be failing for you even though the apple target succeeds on travis?

Seems like the reason is that travis doesn't build tests for the apple target (it is just running make deploy).

I also think I see the reason for the error. BITCOIN_MM files seem to get directly linked into the qt/bitcoin-qt executable instead of being part of qt/libbitcoinqt.a. I will try to move them there.

promag · 2017-11-30T21:55:32Z

I can dig a little to see if I can find a fix.

ryanofsky

Updated 55fe983 -> b67d524 (pr/apptest.5 -> pr/apptest.6) to fix the link error. I was able to reproduce it by building test_bitcoin-qt in a travis docker image, and confirmed that it now links.

src/qt/bitcoingui.cpp

src/qt/test/apptests.cpp

jb55

Linux Tested ACK b67d524

laanwj · 2018-01-30T10:29:56Z

Travis was passing, but I just re-triggered it (to test it on top of master), now https://travis-ci.org/bitcoin/bitcoin/jobs/309721182 :(

Strange:

0.01s$ if [ "$CHECK_DOC" = 1 -a "$TRAVIS_EVENT_TYPE" = "pull_request" ]; then contrib/devtools/lint-all.sh; fi
contrib/devtools/lint-python.sh: 10: contrib/devtools/lint-python.sh: flake8: not found
^---- failure generated from contrib/devtools/lint-python.sh

maflcko · 2018-01-30T12:51:09Z

@laanwj It is a travis bug :( They use the source code from master (merged with this pull) but the .travis.yml of only the pull...

Needs rebase

Sjors · 2018-11-20T14:54:08Z

Tests pass for me on macOS 10.14.1, with and without --disable-bip70.

Shouldn't this be run as part of make check? Otherwise, maybe add them to Travis in a followup?

maflcko · 2018-12-06T21:19:36Z

Could move the first commit to a separate pull request, since that is what needs rebase all the time?

jonasschnelli · 2019-01-04T06:20:34Z

@ryanofsky: interested to pick this up again?

Move-only commit, no code changes

Add test coverage for Qt initialization code & basic RPC console functionality.

ryanofsky · 2019-01-08T15:53:11Z

@ryanofsky: interested to pick this up again?

There was a minor conflict with #15000 last week, but I am maintaining this (and think it probably could be merged).

jonasschnelli · 2019-01-09T07:48:03Z

Tested ACK ca20b65

I don't ran into problems when running on macOS with QT_QPA_PLATFORM=cocoa, I can see then GUI window opening and closing.

Did also ran without issues on Ubuntu Bionic with minimal and xcb Platform.

@ryanofsky any reasons why – on Ubuntu – I see the window opening/closing during the AddressBookTests but not during the AppTests?

7e4bd19 Add BitcoinApplication & RPCConsole tests (Russell Yanofsky) ca20b65 Move BitcoinApplication to header so it can be tested (Russell Yanofsky) Pull request description: Add test coverage for Qt initialization code & basic RPC console functionality Motivation for this change was a bug in #11603 which existing tests failed to catch. Tree-SHA512: f66546ffc84b8e07679c66a73b265023fbf6a0cb8f24f1606a5fcae2dd3b4dc7b2c6d26c69dedcec53398a26ef17c4d5fb28c055698fa6e45e89aa2995cefe2f

ryanofsky · 2019-01-09T15:30:03Z

@ryanofsky any reasons why – on Ubuntu – I see the window opening/closing during the AddressBookTests but not during the AppTests?

That would be an unexpected bug. I'm not seeing any windows shown by default with 699d0bd (current master) running qt/test/test_bitcoin-qt. I do see windows if I set QT_QPA_PLATFORM=xcb (as expected).

You may want to experiment with unset DISPLAY && src/qt/test/test_bitcoin-qt. This should pass and not show any windows.

Updated regtest chain parameters and tests for the Segwit changes in Namecoin after upstream fab17e8. Removed BIP34Hash from chain parameters as that is never used at all in Namecoin (only the height is). Replaced BEGIN/END macros in auxpow code. At the same time, removed all other code explicitly dealing with endian-ness in favour of methods encoding data independently from the system's byte order. Note that the Qt unit test fails with this commit, but it also fails upstream in Bitcoin in the same way. That is likely due to an upstream bug in bitcoin/bitcoin#11625.

7e4bd19 Add BitcoinApplication & RPCConsole tests (Russell Yanofsky) ca20b65 Move BitcoinApplication to header so it can be tested (Russell Yanofsky) Pull request description: Add test coverage for Qt initialization code & basic RPC console functionality Motivation for this change was a bug in bitcoin#11603 which existing tests failed to catch. Tree-SHA512: f66546ffc84b8e07679c66a73b265023fbf6a0cb8f24f1606a5fcae2dd3b4dc7b2c6d26c69dedcec53398a26ef17c4d5fb28c055698fa6e45e89aa2995cefe2f

Backport bitcoin#11625, bitcoin#15101

7e4bd19 Add BitcoinApplication & RPCConsole tests (Russell Yanofsky) ca20b65 Move BitcoinApplication to header so it can be tested (Russell Yanofsky) Pull request description: Add test coverage for Qt initialization code & basic RPC console functionality Motivation for this change was a bug in bitcoin#11603 which existing tests failed to catch. Tree-SHA512: f66546ffc84b8e07679c66a73b265023fbf6a0cb8f24f1606a5fcae2dd3b4dc7b2c6d26c69dedcec53398a26ef17c4d5fb28c055698fa6e45e89aa2995cefe2f

fanquake added the Tests label Nov 7, 2017

ryanofsky mentioned this pull request Nov 7, 2017

Move RPC registration out of AppInitParameterInteraction #11603

Merged

promag reviewed Nov 7, 2017

View reviewed changes

src/qt/test/apptests.cpp Outdated Show resolved Hide resolved

src/qt/test/apptests.cpp Outdated Show resolved Hide resolved

src/qt/test/apptests.h Outdated Show resolved Hide resolved

ryanofsky force-pushed the pr/apptest branch 4 times, most recently from 5be833c to 4bf5ecd Compare November 7, 2017 23:02

ryanofsky commented Nov 7, 2017

View reviewed changes

src/qt/test/apptests.cpp Outdated Show resolved Hide resolved

src/qt/test/apptests.h Outdated Show resolved Hide resolved

src/qt/test/apptests.cpp Outdated Show resolved Hide resolved

ryanofsky force-pushed the pr/apptest branch 5 times, most recently from 4bf5ecd to e94e344 Compare November 8, 2017 18:33

ryanofsky commented Nov 8, 2017

View reviewed changes

src/qt/test/apptests.cpp Outdated Show resolved Hide resolved

promag reviewed Nov 8, 2017

View reviewed changes

src/qt/bitcoingui.cpp Show resolved Hide resolved

src/qt/test/apptests.cpp Show resolved Hide resolved

src/qt/test/apptests.cpp Show resolved Hide resolved

maflcko added the GUI label Nov 10, 2017

ryanofsky force-pushed the pr/apptest branch from e94e344 to 55fe983 Compare November 28, 2017 11:13

ryanofsky force-pushed the pr/apptest branch from 55fe983 to b67d524 Compare November 30, 2017 21:14

ryanofsky commented Nov 30, 2017

View reviewed changes

src/qt/bitcoingui.cpp Show resolved Hide resolved

src/qt/test/apptests.cpp Show resolved Hide resolved

src/qt/test/apptests.cpp Show resolved Hide resolved

jb55 approved these changes Dec 30, 2017

View reviewed changes

ryanofsky force-pushed the pr/apptest branch 2 times, most recently from 328eb86 to 412fb2d Compare February 2, 2018 16:55

DrahtBot added the Needs rebase label Nov 12, 2018

ryanofsky force-pushed the pr/apptest branch from cb5a114 to e6cb33a Compare November 12, 2018 17:00

DrahtBot removed the Needs rebase label Nov 12, 2018

Sjors mentioned this pull request Dec 6, 2018

gui: Fix boost::signals2::no_slots_error in early calls to InitWarning #14783

Merged

DrahtBot added the Needs rebase label Dec 6, 2018

ryanofsky force-pushed the pr/apptest branch from e6cb33a to 7cca59d Compare December 7, 2018 18:11

DrahtBot removed the Needs rebase label Dec 7, 2018

DrahtBot added the Needs rebase label Jan 3, 2019

ryanofsky added 2 commits January 4, 2019 07:31

Move BitcoinApplication to header so it can be tested

ca20b65

Move-only commit, no code changes

Add BitcoinApplication & RPCConsole tests

7e4bd19

Add test coverage for Qt initialization code & basic RPC console functionality.

ryanofsky force-pushed the pr/apptest branch from 7cca59d to 7e4bd19 Compare January 4, 2019 16:06

DrahtBot removed the Needs rebase label Jan 5, 2019

maflcko merged commit 7e4bd19 into bitcoin:master Jan 9, 2019

UdjinM6 added a commit to dashpay/dash that referenced this pull request Sep 18, 2021

Merge pull request #4410 from PastaPastaPasta/backport-new-wallet-pr2

6751412

Backport bitcoin#11625, bitcoin#15101

bitcoin locked as resolved and limited conversation to collaborators Dec 16, 2021

Add BitcoinApplication & RPCConsole tests #11625

Add BitcoinApplication & RPCConsole tests #11625

Uh oh!

Conversation

ryanofsky commented Nov 7, 2017

Uh oh!

fanquake commented Nov 7, 2017

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ryanofsky left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ryanofsky left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

promag left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

promag commented Nov 9, 2017

Uh oh!

jonasschnelli commented Nov 29, 2017

Uh oh!

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

Uh oh!

Uh oh!

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

Uh oh!

Uh oh!

promag commented Nov 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ryanofsky left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jb55 left a comment

Choose a reason for hiding this comment

Uh oh!

laanwj commented Jan 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

maflcko commented Jan 30, 2018

Uh oh!

Sjors commented Nov 20, 2018

Uh oh!

maflcko commented Dec 6, 2018

Uh oh!

jonasschnelli commented Jan 4, 2019

Uh oh!

ryanofsky commented Jan 8, 2019

Uh oh!

jonasschnelli commented Jan 9, 2019

Uh oh!

ryanofsky commented Jan 9, 2019

Uh oh!

Uh oh!

ryanofsky left a comment •

edited

Loading

ryanofsky commented Nov 30, 2017 •

edited

Loading

ryanofsky commented Nov 30, 2017 •

edited

Loading

promag commented Nov 30, 2017 •

edited

Loading

ryanofsky left a comment •

edited

Loading

laanwj commented Jan 30, 2018 •

edited

Loading