Regression Tests: Migrated rpc-tests.sh to all Python rpc-tests.py #6616

ptschip · 2015-09-01T21:16:00Z

created rpc-tests.py
deleted rpc-tests.sh
deleted tests-config.sh.in
travis.yml points to rpc-tests.py

maflcko · 2015-09-01T21:47:01Z

Don't forget to adjust https://github.com/ptschip/bitcoin/blob/all_python/qa/rpc-tests/README.md#notes and https://github.com/ptschip/bitcoin/blob/all_python/README.md as well

ptschip · 2015-09-01T22:11:50Z

I'll do that

I was wondering if we should move that README.md up to the /qa folder
...I think it would be more visible there. I didn't even know it
existed until you just mentioned it.

On 01/09/2015 2:47 PM, MarcoFalke wrote:

Don't forget to adjust
https://github.com/ptschip/bitcoin/blob/all_python/qa/rpc-tests/README.md#notes
and https://github.com/ptschip/bitcoin/blob/all_python/README.md as well

—
Reply to this email directly or view it on GitHub
#6616 (comment).

theuni · 2015-09-02T02:46:04Z

The hard-coded values are a major step backwards. File existence is a very bad metric considering that many of us often cross-compile. Also, anything that requires a certain PWD is inflexible.

Please re-add a pre-processed config file and import it from rpc-tests.py.

theuni · 2015-09-02T17:08:31Z

@ptschip like this:

diff --git a/configure.ac b/configure.ac
index a524bde..c0a4e54 100644
--- a/configure.ac
+++ b/configure.ac
@@ -878,7 +878,7 @@ AC_SUBST(MINIUPNPC_CPPFLAGS)
 AC_SUBST(MINIUPNPC_LIBS)
 AC_CONFIG_FILES([Makefile src/Makefile share/setup.nsi share/qt/Info.plist src/test/buildenv.py])
 AC_CONFIG_FILES([qa/pull-tester/run-bitcoind-for-test.sh],[chmod +x qa/pull-tester/run-bitcoind-for-test.sh])
-AC_CONFIG_FILES([qa/pull-tester/tests-config.sh],[chmod +x qa/pull-tester/tests-config.sh])
+AC_CONFIG_FILES([qa/rpc-tests/tests_config.py],[chmod +x qa/rpc-tests/tests_config.py])

 dnl boost's m4 checks do something really nasty: they export these vars. As a
 dnl result, they leak into secp256k1's configure and crazy things happen.
diff --git a/qa/rpc-tests/tests_config.py.in b/qa/rpc-tests/tests_config.py.in
new file mode 100644
index 0000000..adc7860
--- /dev/null
+++ b/qa/rpc-tests/tests_config.py.in
@@ -0,0 +1,11 @@
+#!/usr/bin/env python2
+BUILDDIR="@abs_top_builddir@"
+EXEEXT="@EXEEXT@"
+
+# These will turn into comments if they were disabled when configuring.
+@ENABLE_WALLET_TRUE@ENABLE_WALLET=1
+@BUILD_BITCOIN_UTILS_TRUE@ENABLE_UTILS=1
+@BUILD_BITCOIND_TRUE@ENABLE_BITCOIND=1
+
+REAL_BITCOIND=BUILDDIR + '/src/bitcoind' + EXEEXT
+REAL_BITCOINCLI=BUILDDIR + '/src/bitcoin-cli' + EXEEXT

Then in the migrated script you can use it like:

#!/usr/bin/env python2
import tests_config

print tests_config.REAL_BITCOINCLI

ptschip · 2015-09-02T17:19:20Z

duh, oh yeah, that's good, i was wondering how to get rid of
tests.config.sh..

On 02/09/2015 10:09 AM, Cory Fields wrote:

@ptschip https://github.com/ptschip like this:

diff --git a/configure.ac b/configure.ac
index a524bde..c0a4e54 100644
--- a/configure.ac
+++ b/configure.ac
@@ -878,7 +878,7 @@ AC_SUBST(MINIUPNPC_CPPFLAGS)
AC_SUBST(MINIUPNPC_LIBS)
AC_CONFIG_FILES([Makefile src/Makefile share/setup.nsi share/qt/Info.plist src/test/buildenv.py])
AC_CONFIG_FILES([qa/pull-tester/run-bitcoind-for-test.sh],[chmod +x qa/pull-tester/run-bitcoind-for-test.sh])
-AC_CONFIG_FILES([qa/pull-tester/tests-config.sh],[chmod +x
qa/pull-tester/tests-config.sh])
+AC_CONFIG_FILES([qa/rpc-tests/tests_config.py],[chmod +x
qa/rpc-tests/tests_config.py])

dnl boost's m4 checks do something really nasty: they export these vars. As a
dnl result, they leak into secp256k1's configure and crazy things happen.
diff --git a/qa/rpc-tests/tests_config.py.in
b/qa/rpc-tests/tests_config.py.in
new file mode 100644
index 0000000..adc7860
--- /dev/null
+++ b/qa/rpc-tests/tests_config.py.in
@@ -0,0 +1,11 @@
+#!/usr/bin/env python2
+BUILDDIR="@abs_top_builddir@"
+EXEEXT="@EXEEXT@"
+
+# These will turn into comments if they were disabled when configuring.
+@ENABLE_WALLET_TRUE@ENABLE_WALLET=1
+@BUILD_BITCOIN_UTILS_TRUE@ENABLE_UTILS=1
+@BUILD_BITCOIND_TRUE@ENABLE_BITCOIND=1
+
+REAL_BITCOIND=BUILDDIR + '/src/bitcoind' + EXEEXT
+REAL_BITCOINCLI=BUILDDIR + '/src/bitcoin-cli' + EXEEXT

Then in the migrated script you can use it like:

#!/usr/bin/env python2
import tests_config

print tests_config.REAL_BITCOINCLI

—
Reply to this email directly or view it on GitHub
#6616 (comment).

ptschip · 2015-09-03T02:00:30Z

@theuni switched over to tests_config.py and imported the vars directly and the tests run fine on Travis. However the BUILDDIR is not compatible with native windows (ie /c/bitcoin) and is not handled well by python so I had to keep the scripting that finds the builddir. Therefore there are a few options moving forward: 1) Keep the scripting that finds the builddir...I made changes so that it uses the system path rather than the os.path so we don't have to be in the pull-tester folder to run the scripts. (2) I can do an os specific hack to normalize the builddir for native windows (3) if we could somehow find a way to put the native os path into the tests_config.py directly but I don't know how...I'll research this a little more tomorrow, any suggestions are welcome...

ptschip · 2015-09-03T17:41:34Z

@theuni The path for the builddir is now correct for each system. I put a few lines in rpc-tests.py to create the correct path if running in a mingw shell.

I looked into putting it further upstream, in configure.ac when tests_config.py.in gets called, but I don't see we can override the @abs_top_builddir@ with our own function and I'm not sure what we would gain.

maflcko · 2015-09-03T18:07:41Z

qa/rpc-tests/README.md

@@ -37,9 +37,13 @@ Helper functions for creating blocks and transactions.
 Notes
 =====

-You can run a single test by calling `qa/pull-tester/rpc-tests.sh <testname>`.
+You can run any single test by calling qa/pull-tester/rpc-tests.py <testname>


Make sure to get the backticks ` right. (L44 as well)

Also, https://github.com/ptschip/bitcoin/blob/all_python/README.md#automated-testing needs update?

thanks, done both

On 03/09/2015 11:08 AM, MarcoFalke wrote:

In qa/rpc-tests/README.md
#6616 (comment):

@@ -37,9 +37,13 @@ Helper functions for creating blocks and transactions.
Notes

-You can run a single test by calling qa/pull-tester/rpc-tests.sh <testname>.
+You can run any single test by calling qa/pull-tester/rpc-tests.py

Make sure to get the backticks ` right. (L44 as well)

Also,
https://github.com/ptschip/bitcoin/blob/all_python/README.md#automated-testing
needs update?

—
Reply to this email directly or view it on GitHub
https://github.com/bitcoin/bitcoin/pull/6616/files#r38677104.

theuni · 2015-09-04T07:49:55Z

@ptschip I'll be away for a few days, will review when I'm back. There are a few PRs going on parallel to this though (libevent httpd, zmq, etc) that may make sense before merging this anyway, since those things will conflict.

ptschip · 2015-09-04T21:36:02Z

@theuni Have a few good days off. I did manage to fix the issue with the path further upstream by adding a script at the end of configure.ac in order to make the correct path edits when running in mingw .

maflcko · 2015-09-22T10:30:56Z

Looks like the last conflict is #6686. I'd say it's safe to wait until #6686 is merged before you rebase.

jgarzik · 2015-09-22T11:27:01Z

concept ACK

laanwj · 2015-09-27T18:32:31Z

tested ACK
(but needs rebase)

ptschip · 2015-10-01T17:46:38Z

@laanwj @MarcoFalke I added the zmq tests, rebased and Travis is happy.

ptschip · 2015-10-01T17:57:11Z

@laanwj @MarcoFalke just got rid of a little whitespace...should be good after this build finishes.

laanwj · 2015-10-01T18:09:09Z

qa/pull-tester/rpc-tests.py

+    for i in range(len(testScripts)):
+       if (len(opts) == 0 or (len(opts) == 1 and "-win" in opts ) or '-extended' in opts 
+           or testScripts[i] in opts or  re.sub(".py$", "", testScripts[i]) in opts ):
+            print  "Running testscript " + testScripts[i] + "..." 


Nit: I think having the testscript name in bold was nice:

print "Running testscript \033[1m" + testScripts[i] + "\033[0m..."

I've tried, tried double and triple and quadruple escaping that python
usually needs, and also tried the termcolor module. It doesn't work
from python and in a "mingw" shell. It will probably work on a native
unix shell but I can't test that out as I'm on Windows. Any suggestions
welcome...

On 01/10/2015 11:09 AM, Wladimir J. van der Laan wrote:

In qa/pull-tester/rpc-tests.py
#6616 (comment):

+# 'forknotify.py',

'p2p-acceptblock.py',

'mempool_packages.py',
+]

+#Enable ZMQ tests
+if ENABLE_ZMQ == 1:

testScripts.append('zmq_test.py')

+if(ENABLE_WALLET == 1 and ENABLE_UTILS == 1 and ENABLE_BITCOIND == 1):

rpcTestDir = buildDir + '/qa/rpc-tests/'

#Run Tests

for i in range(len(testScripts)):

if (len(opts) == 0 or (len(opts) == 1 and "-win" in opts ) or '-extended' in opts

or testScripts[i] in opts or re.sub(".py$", "", testScripts[i]) in opts ):

print "Running testscript " + testScripts[i] + "..."

Nit: I think having the testscript name in bold was nice:

|print "Running testscript \033[1m" + testScripts[i] + "\033[0m..." |

—
Reply to this email directly or view it on GitHub
https://github.com/bitcoin/bitcoin/pull/6616/files#r40946395.

I've tried, tried double and triple and quadruple escaping that python
usually needs, and also tried the termcolor module. It doesn't work
from python and in a "mingw" shell. It will probably work on a native
unix shell but I can't test that out as I'm on Windows. Any suggestions
welcome...

Ok, no problem, then leave it like this. Escape-code based formatting probably doesn't work on windows at all.
If we want this it needs to be conditional on the terminal type, which is too much bother...

laanwj · 2015-10-01T18:10:08Z

qa/pull-tester/rpc-tests.py

+    for i in range(len(testScriptsExt)):
+        if ('-extended' in opts or testScriptsExt[i] in opts 
+           or re.sub(".py$", "", testScriptsExt[i]) in opts):
+            print  "Running testscript " + testScriptsExt[i] + "..." 


Nit: do we still want to call this "2nd level testscript"?

echo -e "Running \033[1m2nd level\033[0m testscript \033[1m${testScriptsExt[$i]}...\033[0m"

i'll fix that and push.

On 01/10/2015 11:10 AM, Wladimir J. van der Laan wrote:

In qa/pull-tester/rpc-tests.py
#6616 (comment):

#Run Tests

for i in range(len(testScripts)):

if (len(opts) == 0 or (len(opts) == 1 and "-win" in opts ) or '-extended' in opts

or testScripts[i] in opts or re.sub(".py$", "", testScripts[i]) in opts ):

print "Running testscript " + testScripts[i] + "..."

subprocess.call(rpcTestDir + testScripts[i] + " --srcdir " + buildDir + '/src ' + passOn,shell=True)

#exit if help is called so we print just one set of instructions

p = re.compile(" -h| --help")

if p.match(passOn):

sys.exit(0)

#Run Extended Tests

for i in range(len(testScriptsExt)):

if ('-extended' in opts or testScriptsExt[i] in opts

or re.sub(".py$", "", testScriptsExt[i]) in opts):

print "Running testscript " + testScriptsExt[i] + "..."

Nit: do we still want to call this "2nd level testscript"?

|echo -e "Running \033[1m2nd level\033[0m testscript
\033[1m${testScriptsExt[$i]}...\033[0m" |

—
Reply to this email directly or view it on GitHub
https://github.com/bitcoin/bitcoin/pull/6616/files#r40946519.

laanwj · 2015-10-01T18:12:31Z

No need to fix all the nits in this pull, we can do those later, if you don't get around to them - probably more important to get this merged before any other changes to the test runner.

1) created rpc-tests.py 2) deleted rpc-tests.sh 3) travis.yml points to rpc-tests.py 4) Modified Makefile.am 5) Updated README.md 6) Added tests_config.py and deleted tests-config.sh 7) Modified configure.ac with script to set correct path in tests_config.py

jonasschnelli · 2015-10-01T19:00:25Z

Concept ACK.
Keeping the bold output would be nice:

This works for me (and should also work on bash mingw?):

old mode 100644
new mode 100755
index f55c5b6..8c7a6b6
--- a/qa/pull-tester/rpc-tests.py
+++ b/qa/pull-tester/rpc-tests.py
@@ -101,7 +101,7 @@ if(ENABLE_WALLET == 1 and ENABLE_UTILS == 1 and ENABLE_BITCOIND == 1):
     for i in range(len(testScripts)):
        if (len(opts) == 0 or (len(opts) == 1 and "-win" in opts ) or '-extended' in opts 
            or testScripts[i] in opts or  re.sub(".py$", "", testScripts[i]) in opts ):
-            print  "Running testscript " + testScripts[i] + "..." 
+            print  "Running testscript \033[1m" + testScripts[i] + "\033[0m..." 
             subprocess.call(rpcTestDir + testScripts[i] + " --srcdir " + buildDir + '/src ' + passOn,shell=True) 
            #exit if help is called so we print just one set of instructions
             p = re.compile(" -h| --help")

laanwj · 2015-10-01T19:18:22Z

@jonasschnelli That's what I proposed too, it works on Linux and OpenBSD but it doesn't on Win apparently. (outputing junk) So it would have to be conditional.

jonasschnelli · 2015-10-01T19:31:31Z

+#Set colors
+cOn = cOff = ""
+if (not "-win" in opts):
+    cOn = "\033[1m"
+    cOff = "\033[0m"

Then use something like: print "Running testscript " + cOn + testScripts[i] + cOff + "..."

ptschip · 2015-10-01T20:45:52Z

@laanwj I fixed the nit regarding the "2nd level" and left the other for another time. Pushed and all is well...

5467820 Migrated rpc-tests.sh to all python rpc-tests.py (ptschip)

ptschip · 2015-10-01T21:08:08Z

i like the idea, but -win tests will not always be disabled (i hope)...

so it would have to be something operating specific like

+#Set colors
+cOn = cOff = ""
+if (os.name == 'posix'):

cOn = '\033[1m'
cOff = '\033[0m'

(oddly python needs the double \ for a )

If this is ok, i'll add it and push.

On 01/10/2015 12:32 PM, Jonas Schnelli wrote:

+#Set colors
+cOn = cOff = ""
+if (not "-win" in opts):

cOn = "\033[1m"

cOff = "\033[0m"

Then use something like: |print "Running testscript " + cOn +
testScripts[i] + cOff + "..."|

—

btcdrak · 2015-10-01T21:12:51Z

@ptschip This PR is now merged, any new changes would have to be in a new branch + PR

mruddy · 2015-10-03T11:51:17Z

might want to update https://github.com/bitcoin/bitcoin/blob/master/README.md#automated-testing

ptschip · 2015-10-03T15:07:57Z

thanks, that one got missed....updated and new PR created.

On 03/10/2015 4:51 AM, mruddy wrote:

might want to update
https://github.com/bitcoin/bitcoin/blob/master/README.md#automated-testing

—
Reply to this email directly or view it on GitHub
#6616 (comment).

Don't chmod a repository-included file in the configure script, and `tests_config.py` is a module that doesn't need to be executable.

ea70997 build: Remove unnecessary chmods after #6616 (Wladimir J. van der Laan)

This reverts commit 7fd5d4e. We need to un-revert this before pulling in bitcoin/bitcoin#6616

ptschip force-pushed the all_python branch from e7b82c6 to 9f79bb7 Compare September 1, 2015 22:00

ptschip force-pushed the all_python branch from 2563500 to e0730cb Compare September 2, 2015 00:12

ptschip force-pushed the all_python branch 3 times, most recently from 76a1bfc to 28542a0 Compare September 2, 2015 16:55

ptschip force-pushed the all_python branch 2 times, most recently from 164e9d2 to 2f16359 Compare September 3, 2015 01:51

maflcko reviewed Sep 3, 2015
View reviewed changes

ptschip force-pushed the all_python branch from 8588842 to 7daa69d Compare September 4, 2015 21:00

ptschip force-pushed the all_python branch 3 times, most recently from 08563ae to 758f217 Compare September 7, 2015 03:09

laanwj added the Tests label Sep 27, 2015

ptschip force-pushed the all_python branch from 758f217 to 0be00eb Compare October 1, 2015 16:21

ptschip force-pushed the all_python branch from 0be00eb to e8402da Compare October 1, 2015 17:57

laanwj reviewed Oct 1, 2015
View reviewed changes

ptschip force-pushed the all_python branch from e8402da to 5467820 Compare October 1, 2015 18:55

laanwj merged commit 5467820 into bitcoin:master Oct 1, 2015

laanwj added a commit that referenced this pull request Oct 1, 2015

Merge pull request #6616

5ab5dca

5467820 Migrated rpc-tests.sh to all python rpc-tests.py (ptschip)

laanwj added a commit to laanwj/bitcoin that referenced this pull request Oct 5, 2015

build: Remove unnecessary chmods after bitcoin#6616

ea70997

Don't chmod a repository-included file in the configure script, and `tests_config.py` is a module that doesn't need to be executable.

laanwj added a commit that referenced this pull request Oct 5, 2015

Merge pull request #6759

2844b9e

ea70997 build: Remove unnecessary chmods after #6616 (Wladimir J. van der Laan)

paveljanik mentioned this pull request Oct 5, 2015

[Trivial] tests-config.sh is superseded by tests_config.py [skip ci] #6762

Merged

ptschip deleted the all_python branch October 17, 2015 01:31

maflcko mentioned this pull request Nov 2, 2015

build: don't distribute tests_config.py #6930

Merged

dagurval mentioned this pull request Nov 11, 2016

Migrate rpc-tests.sh to rpc-tests.py bitcoinxt/bitcoinxt#170

Merged

jnewbery mentioned this pull request Jan 31, 2017

Improve rpc-tests.py #9657

Merged

str4d mentioned this pull request Mar 24, 2017

libevent-based http server zcash/zcash#2176

Merged

str4d added a commit to str4d/zcash that referenced this pull request Mar 25, 2017

Revert "Revert "rpc-tests: re-enable rpc-tests for Windows""

47855b5

This reverts commit 7fd5d4e. We need to un-revert this before pulling in bitcoin/bitcoin#6616

str4d mentioned this pull request Nov 19, 2020

Backport migration from rpc-tests.sh to rpc-tests.py zcash/zcash#4874

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Regression Tests: Migrated rpc-tests.sh to all Python rpc-tests.py #6616

Regression Tests: Migrated rpc-tests.sh to all Python rpc-tests.py #6616

Uh oh!

Conversation

ptschip commented Sep 1, 2015

Uh oh!

maflcko commented Sep 1, 2015

Uh oh!

ptschip commented Sep 1, 2015

Uh oh!

theuni commented Sep 2, 2015

Uh oh!

theuni commented Sep 2, 2015

Uh oh!

ptschip commented Sep 2, 2015

Uh oh!

ptschip commented Sep 3, 2015

Uh oh!

ptschip commented Sep 3, 2015

Uh oh!

maflcko Sep 3, 2015

Choose a reason for hiding this comment

Uh oh!

ptschip Sep 3, 2015

Choose a reason for hiding this comment

@@ -37,9 +37,13 @@ Helper functions for creating blocks and transactions. Notes

Uh oh!

theuni commented Sep 4, 2015

Uh oh!

ptschip commented Sep 4, 2015

Uh oh!

maflcko commented Sep 22, 2015

Uh oh!

jgarzik commented Sep 22, 2015

Uh oh!

laanwj commented Sep 27, 2015

Uh oh!

ptschip commented Oct 1, 2015

Uh oh!

ptschip commented Oct 1, 2015

Uh oh!

laanwj Oct 1, 2015

Choose a reason for hiding this comment

Uh oh!

ptschip Oct 1, 2015

Choose a reason for hiding this comment

Uh oh!

laanwj Oct 1, 2015

Choose a reason for hiding this comment

Uh oh!

laanwj Oct 1, 2015

Choose a reason for hiding this comment

Uh oh!

ptschip Oct 1, 2015

Choose a reason for hiding this comment

Uh oh!

laanwj commented Oct 1, 2015

Uh oh!

jonasschnelli commented Oct 1, 2015

Uh oh!

laanwj commented Oct 1, 2015

Uh oh!

jonasschnelli commented Oct 1, 2015

Uh oh!

ptschip commented Oct 1, 2015

Uh oh!

ptschip commented Oct 1, 2015

Uh oh!

btcdrak commented Oct 1, 2015

Uh oh!

mruddy commented Oct 3, 2015

Uh oh!

ptschip commented Oct 3, 2015

Uh oh!

Uh oh!

@@ -37,9 +37,13 @@ Helper functions for creating blocks and transactions.
Notes