Rework error handling

[erlend-aasland]

• Introduce ClinicError and ClinicWarning
• fail() raises ClinicError
• warn() uses warnings.warn(msg, ClinicWarning)
• the CLI runs main(), catches ClinicError, formats the error message prints to stderr and exits with an error
• adapt the test suite to work with ClinicError

[erlend-aasland]

@AlexWaygood, what do you think of this approach?

[erlend-aasland]

Can be broken up like this:

• PR 1: split out create_cli()
• PR 2: add and use ClinicWarning
• PR 3: add and use ClinicError (including test adjustments)

[erlend-aasland]

(BTW, since I touched pretty much every test that checks failure of some kind, I just normalised variable naming for them all)

[AlexWaygood]

I have a few more thoughts after you've taken a look at my first round of review ;)

But again, nothing major!

[erlend-aasland]

Since you agree with the overall approach, let's take this to the cpython repo in smaller steps :)

[AlexWaygood]

Yep. One more thing I would say is that it would be nice to restructure the top-level if __name__ == "__main__" block like this (renaming your existing main() function in this PR to be a run_clinic() function):

def main(argv: list[str] | None = None) -> None:
    cli = create_cli()
    try:
        args = cli.parse_args(argv)
        run_clinic(args)
        sys.exit(0)
    except CLIError as exc:
        if msg := str(exc):
            sys.stderr.write(f"Usage error: {msg}\n")
        cli.print_usage()
        sys.exit(1)
    except ClinicError as exc:
        msg = textwrap.dedent(f"""\
            Error in file {exc.filename!r} on line {exc.lineno}:
            {exc}
        """)
        sys.stderr.write(str(exc))
        sys.exit(1)

if __name__ == "__main__":
    main()

(ArgumentParser.parse_args() takes an optional parameter -- if a list of strings is provided to that parameter, it will parse those args instead of sys.argv.)

If we did that, it would be very easy to do end-to-end tests of the CLI without using subprocesses. (The tests got a fair bit slower for me locally on Windows since we started using subprocesses -- they have a lot of overhead on Windows, sadly 😞). When the CLI is actually run, cli.parse_args() will use sys.argv, but in tests, we can pass arbitrary arguments to the main() function to accurately mock a real run of the CLI.

[erlend-aasland]

If we did that, it would be very easy to do end-to-end tests of the CLI without using subprocesses. (The tests got a fair bit slower for me locally on Windows since we started using subprocesses -- they have a lot of overhead on Windows, sadly 😞). When the CLI is actually run, cli.parse_args() will use sys.argv, but in tests, we can pass arbitrary arguments to the main() function to accurately mock a real run of the CLI.

That's a very good idea. Reducing CI (and local dev workflow) time is a very good thing.

[erlend-aasland]

https://github.com/python/cpython/pull/107551