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

test: Add tests for tableView in AddressBookPage dialog #591

Merged
merged 2 commits into from
May 9, 2022

Conversation

hebasto
Copy link
Member

@hebasto hebasto commented Apr 23, 2022

This PR is a prerequisite for more thorough testing of filtering in the AddressBookPage class in context of #578 and #585.

Required for #592.

@hebasto hebasto added the Tests label Apr 23, 2022
@hebasto
Copy link
Member Author

hebasto commented Apr 23, 2022

Friendly ping @promag @prusnak

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK

in edae3ab, we can do this because when not specifying the connection type; there is an invokeMethod overload that uses Qt:AutoConnection. In this context Qt::AutoConnection will really be Qt::QueuedConnection1 because the receiver and emitter are in different threads.

https://doc.qt.io/qt-5/qt.html#ConnectionType-enum

Copy link
Contributor

@promag promag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code review ACK 1506913.

@@ -369,7 +369,7 @@ static void NotifyAddressBookChanged(WalletModel *walletmodel,
QString strPurpose = QString::fromStdString(purpose);

qDebug() << "NotifyAddressBookChanged: " + strAddress + " " + strLabel + " isMine=" + QString::number(isMine) + " purpose=" + strPurpose + " status=" + QString::number(status);
bool invoked = QMetaObject::invokeMethod(walletmodel, "updateAddressBook", Qt::QueuedConnection,
bool invoked = QMetaObject::invokeMethod(walletmodel, "updateAddressBook",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

edae3ab

Could replace with lambda while touching this code.

@hebasto hebasto merged commit 3dd95cb into bitcoin-core:master May 9, 2022
@hebasto hebasto deleted the 220423-test-ab branch May 9, 2022 20:21
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 9, 2022
…ssBookPage` dialog

1506913 qt, test: Add tests for `tableView` in `AddressBookPage` dialog (Hennadii Stepanov)
edae3ab qt: No need to force Qt::QueuedConnection for NotifyAddressBookChanged (Hennadii Stepanov)

Pull request description:

  This PR is a prerequisite for more thorough testing of filtering in the `AddressBookPage` class in context of bitcoin-core/gui#578 and bitcoin-core/gui#585.

  Required for bitcoin-core/gui#592.

ACKs for top commit:
  promag:
    Code review ACK 1506913.

Tree-SHA512: 86986d47606cbd54d813436c7afb21894e2200b6d3042a7aa0b5e84821c765bd68b14ad38a445069891ab33f2d7bcd4933b8373e14e9afb0c91f1a6ddf4da740
@bitcoin-core bitcoin-core locked and limited conversation to collaborators May 9, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants