Do we want the same behavior on L44?

wait_for_verack() is called by the comparison test framework at the start of the test run (comptest.py line 300).

Is there any work we can do in-between this and the wait for verack?

Maybe have a comment here about having to wait for verack rather than just deleting the comment.

No, we need to wait for the network thread to start before receiving the verack (the network thread is what catches the verack and calls the callback).

Is there a more idiomatic way to get the dirname?

Not that I know of.

I don't like the fact that this test is using shutil and removing files itself. This change just makes it slightly less damaging (ie it's only removing the datadir rather than the entire tmpdir)

Maybe raise the error yourself rather than assert, unless you only want it in debug mode?

I don't think there's any difference. No-one runs the test cases with -O

using asserts as control flow is generally an antipattern (I think that the for/else loop is a bit clearer in any case).

ah. I see what you're saying now. Yes, I'll remove the assert from the control flow.

Maybe a bit cleaner to do something like (a couple of changes, feel free to pick and choose -- I was torn on the os.path.exists+getsize v.s. continual reading with open).

        for t in xrange(100):
            if os.path.exists(self.alert_filename) and os.path.getsize(self.alert_filename):
                break
            if t != 99: time.sleep(0.1) 
        else:
            raise AssertionError("-alertnotify did not warn of up-version blocks")
        with open(self.alert_filename, 'r', encoding='utf8') as f:
            alert_text = f.read()

I think this is equivalent.

yeah; mostly this was motivated by being able to get rid of the stateful timeout = 10; assert timeout > 0; timeout -= 0.1 combo.
This is also fine.

        for t in xrange(100):
            with open(self.alert_filename, 'r', encoding='utf8') as f:
                alert_text = f.read()
            if alert_text:
                break
            time.sleep(0.1) 
        else:
            raise AssertionError("-alertnotify did not warn of up-version blocks")

This should rarely actually take 10 seconds, if the code is correct, right? Maybe 1 second will be a bit better in the case where it is actually broken & you're trying to fix it.

Depends. Travis catches a lot of timing edge cases. I don't think there's any problem making the timeout something high like 10 seconds since the while loop will break as soon as the file is written.

@jnewbery nit: Any reason you changed the raise AssertionError to assert False?

No good reason. Equivalent behaviour but you're probably right that raise AssertionError is better style here.