utAck, seems useful!

Yes, makes sense. I should have included something like this when I added the json tests.

I think #9023 (which outputs a contextual diff for failing test cases) should make it much easier to track down formatting errors, since it'll point you to the exact line that's different.

However, I think this is still useful for verifying that the output is indeed valid json.

Only nit is that compare_output() can raise errors which aren't caught anywhere and don't result in useful logging. For example, here's the logging if the output file is invalid json:

~/bitcoin/src$ test/bitcoin-util-test.py --srcdir='.'
Traceback (most recent call last):
  File "test/bitcoin-util-test.py", line 32, in <module>
    bctest.bctester(srcdir + "/test/data", "bitcoin-util-test.json", buildenv, verbose = verbose)
  File "/home/vagrant/bitcoin/src/test/bctest.py", line 72, in bctester
    bctest(testDir, testObj, buildenv.exeext)
  File "/home/vagrant/bitcoin/src/test/bctest.py", line 50, in bctest
    if not compare_output(outs[0], outputData, outputType):
  File "/home/vagrant/bitcoin/src/test/bctest.py", line 14, in compare_output
    match = (json.loads(a) == json.loads(b))
  File "/usr/lib/python2.7/json/__init__.py", line 338, in loads
    return _default_decoder.decode(s)
  File "/usr/lib/python2.7/json/decoder.py", line 366, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/usr/lib/python2.7/json/decoder.py", line 382, in raw_decode
    obj, end = self.scan_once(s, idx)
ValueError: Invalid control character at: line 3 column 14 (char 95)

It'd be nice to wrap the json calls in try/except, and maybe have the final else branch of compare_output() print an error and exit rather than raise an error.

Yea, that error 'Invalid control character at: line 3 column 14 (char 95)" is actually very good and specific. The only thing missing is context of what file it occurs in. Would make sense to print it with the filename.

I think #9023 (which outputs a contextual diff for failing test cases) should make it much easier to track down formatting errors, since it'll point you to the exact line that's different.

I too think #9023 is useful, and we should do that too, however this solves a different issue: say the JSON library is changed to pretty-print slightly differently. JSON itself is defined by a standard, but pretty-printing is not. This verifies separately that the functional part (the data) still matches. Once you now that, you can harmlessly copy the new formatting if you agree with it.

• Improved error reporting:

Error parsing expected output blanktx.json as json: Expecting property name: line 4 column 18 (char 179)
Error parsing expected output blanktx.hex as hex: Non-hexadecimal digit found
Error parsing command output as hex: Non-hexadecimal digit found

• Replaced rstrip with strip

Looks good. Tested ACK

utACK 6c5cd9d