Replaces #11779 . Notes from that PR:
Currently, when a functional test fails, the debug logs are printed sequentially to the travis log. This makes debugging race conditions based on the travis log hard. Instead, all logs events should be combined and sorted by their timestamp, then appended to the travis log. @MarcoFalke
ConceptACK. The --combinedlogslen option seems quite useful outside of Travis as well.
Instead of calling combine_logs from test_runner when the environment variable
PYTHON_DEBUG is set, add a combinedlogslen argument. This makes it easier
to call when running locally, and allows a different length of combined logs
to be printed.
330 | + # Print the final `combinedlogslen` lines of the combined logs 331 | + print('{}Combine the logs and print the last {} lines ...{}'.format(BOLD[1], combined_logs_len, BOLD[0])) 332 | + print('\n============') 333 | + print('{}Combined log for {}:{}'.format(BOLD[1], testdir, BOLD[0])) 334 | + print('============\n') 335 | + subprocess.run(["{} -c {} | tail -{}".format(tests_dir + './combine_logs.py', testdir, combined_logs_len)], shell=True, universal_newlines=True)
Sorry I missed this when looking at that branch initially, but I doubt this will work on Windows.
Since there are plans to work on native Windows builds (https://github.com/bitcoin/bitcoin/pull/11526#issuecomment-345606323) and hopefully tests, I'd prefer if we avoided the code churn.
Also, subprocess.run was introduced in python 3.5. However, our supported version is 3.4
yeah - I noticed that too :(
Pushed a new version which I think should fix both problems:
subprocess.Popen().communicate()shell=True argument and actually pass in arguments to the subprocess properly
utACK ff8a9b068ae15c569ce66abb58bf6f909b879a7d.
Should also work on travis: C.f. the red ones in https://travis-ci.org/MarcoFalke/bitcoin/builds/309170074
Thanks @MarcoFalke - good to know!
I haven't tested this on windows and I don't have a windows test environment to hand. Is someone able to do that?
@jnewbery We are using some non-windows code in other places, so I'd be surprised to see it running on Windows with current master.
I'm fairly sure that this way of launching another python script does not work on windows (because there is no #! interpreter handling). But yeah, as we don't run Travis on windows, I don't think it should prevent this from getting merged. Easy enough to just avoid the option if it doesn't work if you run manually.
utACK ff8a9b0
I'm fairly sure that this way of launching another python script does not work on windows (because there is no #! interpreter handling)
Good point. I think that's right, which I think means that test_runner.py wouldn't work at all on windows - the tests themselves are invoked in the same way.
If we want to modify this to work on windows I think we'd need to modify all those calls to push python to the head of the args list in all those calls to subprocess
If we want to modify this to work on windows I think we'd need to modify all those calls to push python to the head of the args list in all those calls to subprocess
Right. Handling this in a fully portable way is non-trivial - you'd have to figure out what the current interpreter is, and start the script with that. Not sure what the canonical way is. Would warrant an utility function in itself.
Python launcher for windows: https://docs.python.org/3/using/windows.html#python-launcher-for-windows should help. If the test_runner is launched with the launcher and all the scripts have the same shebang, then I think they'd use the same interpreter.
Milestone
0.16.0