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: Use Regex Search in Apptests #337

Merged
merged 2 commits into from
Aug 5, 2021

Conversation

jarolrod
Copy link
Member

This PR refactors our GUI apptests so that it uses regex search to find values in our console/qtextedit output regardless if it is in plaintext, html, or markdown.

This introduces a new function FindInConsole which uses QRegularExpression to search the output of the console. The function must be provided with a perl compatible regex pattern which wants to match a single group. The function then returns the matched group. If no match is found, an empty QString is returned.

We then use this new function in TestRpcCommand to find the current chain value instead of reading with univalue.

This approach can apply to a wider variety of testing scenarios as we can reuse this function to search for values when the console output is exported in a different format than plaintext. As an example, A follow up PR will add tests for console resizing and needs to look for the size in html tags after exporting the console text with toHtml().

@hebasto hebasto added the Tests label May 17, 2021
@jarolrod jarolrod force-pushed the regex-search-test branch from d32b704 to d342630 Compare May 18, 2021 14:32
@jarolrod
Copy link
Member Author

updated from d32b704 -> d342630

Changes:

  • Address CI failure by explicitly making the "regtest" string a QString
    • Note: Using QStringLiteral would be more efficient since it avoids constructing a QString and copying data to it. But we are not in a loop, the performance benefit is minimal, and just QString is cleaner.
  • While here, include missing QString header

@@ -22,6 +22,7 @@
#include <QAction>
#include <QEventLoop>
#include <QLineEdit>
#include <QRegularExpression>
Copy link
Member

@hebasto hebasto May 20, 2021

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

I verified that Qt itself uses QRegularExpression header, at least for version we use in depends.

So, I'm ok with it.

@jarolrod
Copy link
Member Author

updated from d342630 -> 1a63f44 (pr337.01 -> pr337.02)

Changes:

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

Approach ACK 1a63f44.

src/qt/test/apptests.cpp Outdated Show resolved Hide resolved
src/qt/test/apptests.cpp Outdated Show resolved Hide resolved
@jarolrod jarolrod force-pushed the regex-search-test branch from 1a63f44 to 9bde381 Compare May 26, 2021 04:27
@jarolrod
Copy link
Member Author

updated from 1a63f44 -> 9bde381 (pr337.02, pr337.03, diff)

Changes:

jarolrod added 2 commits June 1, 2021 02:07
Allows for regex searching into the console output.
use the FindInConsole function to regex search for values in apptests instead of Univalue read.
@jarolrod jarolrod force-pushed the regex-search-test branch from 9bde381 to 6969b2b Compare June 1, 2021 06:22
@jarolrod
Copy link
Member Author

jarolrod commented Jun 1, 2021

Updated from 9bde381 -> 6969b2b (pr337.03 -> pr337.04)

Changes:

Copy link
Member

@hebasto hebasto left a comment

Choose a reason for hiding this comment

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

ACK 6969b2b

Copy link
Contributor

@shaavan shaavan left a comment

Choose a reason for hiding this comment

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

ACK 6969b2b
Compiled the PR properly on Ubuntu 20.04

The PR introduces regex search functionality to apptest.cpp file by replacing univalue based search, which requires the qmessagewidget to be output in plaintext to work properly, with a more general Qregularexpression based search which doesn’t explicitly require a plaincheck to function.

Tested if the new apptest is running properly by running ./src/qt/test/test_bitcoin-qt and from there obtained: PASS : AppTests::appTests()
For the sake of completion, the final result obtained from running the test_bitcoin-qt file was:
Totals: 3 passed, 0 failed, 0 skipped, 0 blacklisted, 96ms

This PR is an improvement over the master, as now the search string need not have to be a plaintext, which increases the number of use cases of the search action. An example of an additional use case is if we want to output the qmessagewidget in HTML to examine the values of the current HTML elements.

@hebasto hebasto merged commit be37037 into bitcoin-core:master Aug 5, 2021
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 5, 2021
6969b2b qt, test: use regex search in apptests (Jarol Rodriguez)
d09d1cf qt, test: introduce FindInConsole function (Jarol Rodriguez)

Pull request description:

  This PR refactors our GUI `apptests` so that it uses regex search to find values in our console/qtextedit output regardless if it is in `plaintext`, `html`, or `markdown`.

  This introduces a new function `FindInConsole` which uses [QRegularExpression](https://doc.qt.io/qt-5/qregularexpression.html) to search the output of the console. The function must be provided with a [perl compatible regex](https://www.debuggex.com/cheatsheet/regex/pcre) pattern which wants to match a single group. The function then returns the matched group. If no match is found, an empty `QString` is returned.

  We then use this new function in `TestRpcCommand` to find the current `chain` value instead of reading with univalue.

  This approach can apply to a wider variety of testing scenarios as we can reuse this function to search for values when the console output is exported in a different format than `plaintext`. As an example, A follow up PR will add tests for console resizing and needs to look for the size in `html` tags after exporting the console text with `toHtml()`.

ACKs for top commit:
  hebasto:
    ACK 6969b2b
  ShaMan239:
    ACK 6969b2b

Tree-SHA512: 4db8bcd4a1acc4539ca64bbd7de572fe7dd6afc3e95108235abfc2891585bc4db3a56a33928fa38e8d44ac87023ce0dee3abcfadfbcd4440e3a21a52fef02536
@bitcoin-core bitcoin-core locked as resolved and limited conversation to collaborators Aug 16, 2022
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.

4 participants