Make --strict default and remove parameter

[datapythonista]

I think asv interface can be improved by making --strict parameter of the run command to be the default. It makes the command return an exit code != 0 if things break, which I think it's the behavior that user would expect.

I'll also remove the parameter, since ignoring errors can be simply done as asv | true.

[trexfeathers]

Error 'catching' at the benchmark level - rather than the command line level - is useful for distinguishing between problems in:

• Run execution (environment building, benchmark discovery etc.)
• Benchmark execution

Can you suggest any alternative ways of making this distinction that don't involve just grepping for certain text (an anti-pattern in my book)? I have use cases where AFAICT I'd effectively have to do pandas-dev/pandas#46633 in reverse!

[datapythonista]

We can simply use an exit code of 1 for an error in asv, and an exit code 2 for errors in benchmarks. I don't see the need to distinguish those automatically often, I assume most project will just want a red build if things fail. But just standard bash and checking $? would work when someone wants to allow benchmarks failure (which if I'm not missing something, it's not a great thing to do anyway).

[trexfeathers]

That would work great, thanks. Just thought some more and I think the below examples would no longer work with what you are proposing.

This is useful in our case for speeding up debugging. Many of our benchmarks need to be given access to various paths via env vars. If I just want to confirm run behaviour (env setup, repeat number, interleaving etc.) then setting up takes literally half as long if I don't need to make sure the benchmarks themselves will run perfectly.

We also have a couple of scalability benchmarks that use a lot of memory, in which case failures are often useful 'data' (ran out of memory) and we want the run to keep going.

Hope that's helpful detail.

[datapythonista]

I don't understand your use case, sorry. How do you test that the environment setup, repeat number... work fine? The run time should be the same regardless of the status code the process returns, this sounds unrelated to this flag.

First thing is to make sure --strict=true by default. I think asv run having broken benchmarks and causing a green CI is misleading, and can cause errors fail silently, which is not great.

So, the rest of the discussion here is about whether --strict (or --no-strict) is worth keeping. Asv commands have a huge number of parameters, and anything that can be simplified in the code would also be great. And as said, this would still be possible using $?. If there are use cases so common that lots of projects will need to implement the extra bash to handle $?, fine to keep strict. But all what you're saying seems unrelated to this. The run will take the same and will keep going, regardless of the exit code.

[trexfeathers]

I think asv run having broken benchmarks and causing a green CI is misleading, and can cause errors fail silently, which is not great.

Agreed

Asv commands have a huge number of parameters, and anything that can be simplified in the code would also be great.

Agreed

The run will take the same and will keep going, regardless of the exit code.

Thanks, I had been assuming that --strict would cause an immediate exit as soon as there was a benchmark failure. Sorry for the misunderstanding - I've only worked with --strict a few times so far.

If --strict (or a future default behaviour) just causes a failure exit code at the end of the run then it would indeed be easy enough to do a test run, or to embed in a larger script that publishes whatever results worked, even if some benchmarks failed. It would definitely be useful to have the two different exit codes that you suggested.

[datapythonista]

Ah, cool. That's why I wasn't understanding you. Afaik there is nothing to cancel the build early.

--strict is just to return 2 instead of 0 if there are failures: https://github.com/airspeed-velocity/asv/blob/master/asv/commands/run.py#L518