Replace ScriptRunner with pytest-console-scripts fixtures.

[HexDecimal]

This resolves one of the tasks in #125

I'm throwing out scriptrunner.py. Any desired features can be implemented upstream. I've added my own protocols to get around its missing typing.

Updated type-hints and asserts for all the scripts tests. There was more I wanted to do but my changes were beginning to reach into other modules.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f639c4d) 94.93% compared to head (9b07f10) 94.93%.

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #185   +/-   ##
=======================================
  Coverage   94.93%   94.93%           
=======================================
  Files          14       14           
  Lines        1125     1125           
=======================================
  Hits         1068     1068           
  Misses         57       57           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[matthew-brett]

Looks good - did you check that the script tests fail if the scripts are broken in some way?

[HexDecimal]

Looks good - did you check that the script tests fail if the scripts are broken in some way?

This does what the old script runner was doing but it's maintained upstream and the new script runner is well tested. I've even added my own features and tests (and of course full type-hinting) to the fixture upstream.

Some tests were exact so I'm not sure what you mean by breaking them. Scripts which were expected to fail before still fail as expected as their returncode is checked. There's some changes with the new outputs since this returns strings like subprocess.run instead of lists of strings which were common with the old runner and that will have changed the tests a little.

[matthew-brett]

Sorry - I only meant - did you try breaking the code for the scripts and running the tests to make sure they fail in that circumstance?

[HexDecimal]

did you try breaking the code for the scripts and running the tests to make sure they fail in that circumstance?

I didn't do that. All of the Delocate script tests use MacOS linkage so I can't break them locally. The tests were all broken when the new script runner wasn't returning lists of strings anymore, but that was mostly a process of making the type checker happy rather than waiting on the CI to give me test results. Then after that the tests still pass and test coverage hasn't even changed despite me deleting delocate/tests/scriptrunner.py, so I see that as the new fixture being a perfect replacement for the bundled one.

I did notice that run_command checks the return code and raises on failure so I made sure that check=True was added to the new fixtures, but that was a new feature I just added upstream after starting this PR. There are already scripts which cause a failure and raise exceptions and those were already tested but I went and cleaned up the code more.

If there's anything more specific you want to try then you're in a better position than I am to prod at it.

[HexDecimal]

I've tested "breaking the code for the scripts" and the results are as expected: tests fail with verbose output of the failing command including the error code, stdout, and stderr.

[matthew-brett]

Thanks for this ...

[HexDecimal]

I assume you're too busy to handle these PR's? This PR was blocking me from working on more script related tasks. I'd like to merge 2 other PR's as well (the ones that were requested, which I've self-assigned.)

[matthew-brett]

Yes, sorry - I'm mid-term and very busy at the moment.