Fix tox tests so they will actually fail

[AdamWill]

As things stand, the tests never fail when run via tox.
__main__.py runs each individual test script, but always exits
0, even if a test script exits >0.

Signed-off-by: Adam Williamson awilliam@redhat.com

[AdamWill]

It kinda looks like at some point, it was the job of the little bash script in tox.ini to loop over each individual test and handle failures, then someone did half a job of moving that to __main__.py but didn't include the bit that handles failures, and left the bash loop in tox.ini even though it's now entirely pointless.

[AdamWill]

Updated to clarify this applies to how tox runs the tests. Now I see that they're run differently in CI. (That seems suboptimal; I set up my projects so that CI runs tox, that way you can run the tests locally exactly the way CI runs them...)

[mmckerns]

@AdamWill: dill is an old project and some infrastructure is a decade old. I just recently made sure that dill complies with modern PEP build requirements and usage/install patterns... and am in the process of updating all the "old" hacks that were build when there wasn't support for what was needed. Running the tests (in CI, with main, and with tox) is definitely one of those places that needs a little updating.

[AdamWill]

Yeah, I know it's a lot of work. Ironically, this is step six in a yak shave that started out with me cleaning up the infrastructure in one of my projects...:D

[mmckerns]

You prefer the test results are all displayed at the end (i.e. flush=False)? Arguably it's a bit cleaner when there's other output -- however, other output really should not be there...

[AdamWill]

That wasn't intentional, honestly, I think I was basing off an older version of the existing file (I was jumping around between main and 0.3.5.1 a lot for packaging reasons).

I do think it's maybe better, though. Two of the tests can produce output, I believe - test_selected and test_session.

[mmckerns]

I think the more appropriate thing is to leave it with flush=True... and then let a later PR clean up the spurious output. So, if you can change the value of flush, that'd be great, and I'll merge it.

[AdamWill]

Done. For the record, it was actually changed to use flush=True in 260b372 , but without any explanation in the commit message as to why. It may just have been something Leonardo did locally while he was working on the test fix and inadvertently left in the commit?

[mmckerns]

Thanks! It was due to be added after we dropped support for 2.7, but it didn't have it's own PR.

[leogama]

@AdamWill, I really didn't make any comment on this change as it was very minor. The print(.) statement in tests is a minimalist progress indicator. However, without the flush option the dots accumulate in the buffer and are printed out just at the end, making the tests seem like they are hanging (at least on my cheap notebook).