[Qt] misc PaymentServer changes (e.g. changes to eventFilter()) #3254

Diapolo commented at 6:24 PM on November 14, 2013: none

make eventFilter() private and pass events on to QObject::eventFilter() instead of just returning false
re-work paymentservertest.cpp to correctly handle the event test after the above change (rewrite test_main to allow usage of QCoreApplication:: in the tests)
delete socket when we were unable to connect in ipcSendCommandLine()
show a message to the user if we fail to start-up (instead of just a debug.log entry)
misc small comment changes

Diapolo commented at 7:42 PM on November 14, 2013: none

@laanwj Any idea what is causing this build error, I don't understand the problem.

Edit: Was caused by paymentservertest.cpp, because Gavin called eventFilter() directly, which isn't possible anymore after this pull.

in src/qt/test/paymentservertests.cpp:None in 762efac2cd outdated

  40 | @@ -41,9 +41,8 @@ static SendCoinsRecipient handleRequest(PaymentServer* server, std::vector<unsig
  41 |      f.write((const char*)&data[0], data.size());
  42 |      f.close();
  43 |  
  44 | -    // Create a FileOpenEvent and send it directly to the server's event filter:

laanwj commented at 6:35 AM on November 18, 2013:

This is meant to be a test of the event filter, don't replace it with a direct call to handleURIOrFile. Aren't there other ways to submit events to an object?

Diapolo commented at 7:03 AM on November 18, 2013:

I didn't really care to investigate, as it was already an ugly construct before... do you prefer to keep this (and make eventFilter public again) or to find a better way?

laanwj commented at 7:46 AM on November 18, 2013:

The only objective here is to deliver the event to the object, yes please find another way no matter how ugly it would be in real code, it's a test so that's allowed.

Diapolo commented at 8:47 AM on November 18, 2013:

I hope that new code works ;).

in src/qt/test/paymentservertests.cpp:None in 48ad6e940f outdated

  46 | +    // and send a file open event to the object
  47 | +    QObject object;
  48 | +    object.installEventFilter(server);
  49 |      QFileOpenEvent event(f.fileName());
  50 | -    server->eventFilter(NULL, &event);
  51 | +    QCOMPARE(QCoreApplication::sendEvent(object, event), true);

laanwj commented at 8:56 AM on November 18, 2013:

Nice solution, hope it works too

Diapolo commented at 9:22 AM on November 18, 2013: none

Damn:

<pre> paymentservertests.cpp: In function SendCoinsRecipient handleRequest(PaymentServer*, std::vector<unsigned char, std::allocator<unsigned char> >&): paymentservertests.cpp:49: error: no matching function for call to QCoreApplication::sendEvent(QObject&, QFileOpenEvent&) /usr/include/qt4/QtCore/qcoreapplication.h:214: note: candidates are: static bool QCoreApplication::sendEvent(QObject*, QEvent*) paymentservertests.cpp:49: error: return-statement with no value, in function returning SendCoinsRecipient </pre>

My fault, the function wants a pointer not an object ^^.

Diapolo commented at 2:16 PM on November 18, 2013: none

I hope this is the last time now -_-.

Diapolo commented at 2:59 PM on November 18, 2013: none

Wohoooo :)

in src/qt/test/test_main.cpp:None in 351f96351a outdated

   9 | @@ -11,6 +10,11 @@ int main(int argc, char *argv[])
  10 |  {
  11 |      bool fInvalid = false;
  12 |  
  13 | +    // Don't remove this, it's needed to access
  14 | +    // QCoreApplication:: in the tests
  15 | +    QCoreApplication app(argc, argv);
  16 | +    app.setApplicationName("Bitcoin-Qt-test");

laanwj commented at 3:37 PM on November 18, 2013:

Good find

Diapolo commented at 6:24 PM on November 18, 2013:

Thanks mate :).

Diapolo commented at 3:03 PM on November 20, 2013: none

@laanwj Merge-ready or anything more to be done here?

[Qt] misc PaymentServer changes (e.g. changes to eventFilter())

- make eventFilter() private and pass events on to QObject::eventFilter()
  instead of just returning false
- re-work paymentservertest.cpp to correctly handle the event test
  after the above change (rewrite test_main to allow usage of
  QCoreApplication:: in the tests)
- delete socket when we were unable to connect in ipcSendCommandLine()
- show a message to the user if we fail to start-up (instead of just a
  debug.log entry)
- misc small comment changes

4cf3411056

in src/qt/paymentserver.cpp:None in 351f96351a outdated

 296 | @@ -291,16 +297,12 @@ bool PaymentServer::ipcSendCommandLine(int argc, char* argv[])
 297 |      google::protobuf::ShutdownProtobufLibrary();
 298 |  }
 299 |  
 300 | -//
 301 | -// OSX-specific way of handling bitcoin: URIs and

laanwj commented at 3:24 PM on November 20, 2013:

Why remove this comment? AFAIK this is correct, only on Mac the URIs and such come in through this way

Diapolo commented at 4:45 PM on November 20, 2013:

By mistake, a former pull extended this function, which made the comment wrong, don't mind, I changed it back.

Diapolo commented at 10:09 AM on December 6, 2013: none

If @BitcoinPullTester is happy, this should be merge-ready @laanwj.

laanwj commented at 10:18 AM on December 6, 2013: member

Agreed

BitcoinPullTester commented at 10:25 AM on December 6, 2013: none

Automatic sanity-testing: PASSED, see http://jenkins.bluematt.me/pull-tester/4cf3411056f6a59fc5fe07784b6b6a512d76b046 for binaries and test log. This test script verifies pulls every time they are updated. It, however, dies sometimes and fails to test properly. If you are waiting on a test, please check timestamps to verify that the test.log is moving at http://jenkins.bluematt.me/pull-tester/current/ Contact BlueMatt on freenode if something looks broken.

laanwj referenced this in commit 9ac11a4e1e on Dec 6, 2013

laanwj merged this on Dec 6, 2013

laanwj closed this on Dec 6, 2013

Diapolo deleted the branch on Dec 6, 2013

Bushstar referenced this in commit 2afdc8c6f6 on Apr 8, 2020

DrahtBot locked this on Sep 8, 2021