Skip to content

[RPC-Tests] add simple way to run rpc test over QT clients #7068

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Dec 14, 2015
Merged

[RPC-Tests] add simple way to run rpc test over QT clients #7068

merged 1 commit into from
Dec 14, 2015

Conversation

jonasschnelli
Copy link
Contributor

Use env variable QT=1.
Example: QT=1 ./qa/rpc-tests/wallet.py --srcdir=src/

The RPC test will be executed over GUI clients.
It pretty cool to visually watch the RPC test slowly gets executed.
Much slower then bitcoind; aims to help testing and improving UI situations.

paveljanik
paveljanik reviewed Nov 20, 2015
View reviewed changes
qa/rpc-tests/test_framework/util.py
args = [ binary, "-datadir="+datadir, "-server", "-keypool=1", "-discover=0", "-rest" ]
if os.getenv("QT", ""):
args[0] = "bitcoin-qt"
args.append("-server")
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding -server twice here? See changed line 219. Thanks @instagibbs.

Copy link
Member

Choose a reason for hiding this comment

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

219 I think you mean

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right. There was one to many. Fixed.

maflcko
maflcko reviewed Nov 21, 2015
View reviewed changes
qa/rpc-tests/test_framework/test_framework.py
@@ -113,6 +113,9 @@ def main(self):
self.add_options(parser)
(self.options, self.args) = parser.parse_args()

if os.getenv("QT", ""):
self.options.noshutdown = True
Copy link
Member

Choose a reason for hiding this comment

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

Any reason to do this? This will hinder executing QT=1 ./qa/pull-tester/rpc-tests.py?

Copy link
Member

Choose a reason for hiding this comment

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

I'd also prefer passing noshutdown, if desired, as an explicit option.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The reason why I added the noshutdown=True is, because i think running a RPC test with the UI needs post-run user based verification (does the GUI looks like expected?). As long as there is no automatic screenshot creating (and maybe auto-comparing) I think the noshutdown flag is appropriate.
The QT=1 env flag is a helper IMO. It's not really necessary because one could also pass the path to bitcoin-qt in the BITCOIND env var.

Copy link
Member

Choose a reason for hiding this comment

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

I think the noshutdown flag is appropriate.

Still, you could just pass it via the command line. On the other hand, there is no way to disable it, right now when using QT=.

Copy link
Member

Choose a reason for hiding this comment

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

Right. There is no -shutdown to override it the other way. I still think it's better not to conflate "want to use GUI" and "don't want to shutdown".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes. Agreed. This is a point.
Fixed.

@gmaxwell
Copy link
Contributor

utACK. Any idea why it's slower?

@jonasschnelli jonasschnelli force-pushed the 2015/11/rpc_tests_qt branch 2 times, most recently from 1457207 to ffe8fa6 Compare November 24, 2015 11:46
@jonasschnelli
Copy link
Contributor Author

@gmaxwell: I haven't analyzed why they are slower, but i assume it's because of the whole GUI/QT overhead.

@jonasschnelli
Copy link
Contributor Author

Maybe I should give some directions where this could go to:
QT has easy functions to create a bitmap (example: PNG File) of a window. If there would be a script that runs through all UI situations and create a bunch of screenshots that could help in testing various platforms and all UI change requests (kick a script that builds a html page with screenshots, scroll through them, verify).
Testing multiplatform UI is complicated and time robbing.
I once used a similar system that could also compare screenshots (% divergence) and where capable of detect unwished UI changes.

@laanwj
Copy link
Member

laanwj commented Nov 25, 2015

utACK. Any idea why it's slower?

With e.g. wallet tests with lots of transactions it's bound to be slower because the UI is doing more in that case, it keeps and maintains its own table of transactions to to do things like sort, as well as draw the screen when core's locks are contended.

@maflcko
Copy link
Member

maflcko commented Nov 25, 2015

utACK ffe8fa6

@laanwj
Copy link
Member

laanwj commented Dec 3, 2015

Needs rebase

@jonasschnelli
Copy link
Contributor Author

Rebased and reduced. There are only two changes now:

  1. Pass -server arg always
  2. Add src/qt to the PATH variable

Tests over QT clients can be run like BITCOIND=bitcoin-qt ./qa/rpc-tests/wallet.py --srcdir=src/.

@laanwj
Copy link
Member

laanwj commented Dec 14, 2015

Awesome, utACK

@laanwj laanwj merged commit 979698c into bitcoin:master Dec 14, 2015
laanwj added a commit that referenced this pull request Dec 14, 2015
@laanwj
Merge pull request #7068
5f3c670 
979698c [RPC-Tests] add option to run rpc test over QT clients (Jonas Schnelli)
@maflcko
Copy link
Member

maflcko commented Dec 14, 2015

Would be nice to have ./qa/pull-tester/rpc-tests.py support this as well so one can run all test with the gui and check if it passes.

@laanwj
Copy link
Member

laanwj commented Dec 14, 2015

Doesn't this simply work?

BITCOIND=bitcoin-qt ./qa/pull-tester/rpc-tests.py

@maflcko
Copy link
Member

maflcko commented Dec 14, 2015

Doesn't this simply work?

Fixed by #7209

luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Dec 28, 2015
@jonasschnelli @luke-jr
[RPC-Tests] add option to run rpc test over QT clients
eeff3e7 
Github-Pull: bitcoin#7068
Rebased-From: 979698c
luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Dec 28, 2015
@jonasschnelli @luke-jr
[RPC-Tests] add option to run rpc test over QT clients
05e9b64 
Github-Pull: bitcoin#7068
Rebased-From: 979698c
luke-jr pushed a commit to luke-jr/bitcoin that referenced this pull request Jan 13, 2016
@jonasschnelli @luke-jr
[RPC-Tests] add option to run rpc test over QT clients
6191a9b 
Github-Pull: bitcoin#7068
Rebased-From: 979698c
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
GUI Tests
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jonasschnelli @gmaxwell @laanwj @maflcko @instagibbs @paveljanik