Codecov upload is attempted if CODECOV_TOKEN is set, regardless of whether --codecov is provided

[adamcunnington-gdt]

@Daverball I've ran into an irritating bug.

If the environment variable, CODECOV_TOKEN is set, even to an empty string, this package will cause upload to ALWAYS be attempted (resulting in failure when the token is not set).

My expectation is that this should only happen if I explicitly pass --codecov (or maybe --codecov-token) as a CLI option. I do not expect the upload to be attempted if I don't pass this.

Right now, the only way I can avoid upload is by explicitly unsetting the environment variable (even setting to empty string is not enough)!

[Daverball]

@adamcunnington-gdt I see what you are talking about.

It is not actually attempting to upload anything. But the validation of parameters happens always, since they can be provided in any order and the defaults of those parameters are set to the contents of the environment variables. The way this happens is determined by pytest, so as a plugin I do not have that much control over it.

This is a bit annoying when you didn't set --codecov. However, if I delay validation of environment variables to the end, the behavior is now no longer consistent with providing the arguments to the command. What I would like to be able to do is to silently fail the arguments, unless --codecov is provided as well and only raise the error at the very end once all arguments have been parsed, although I'm not sure argparse can handle such a case.

What I can do either way is treat an empty string in an environment variable the same as an unset environment variable. I will investigate what can be done in the other cases.

Maybe I will decide that it is fine to delay validation of passed arguments and just print an error message to the log, while still running the tests, however that is maybe not what you want in an CI environment, where you're not going to check the command output. But then again the upload can still fail, without the command outputting an error code currently, since it is a bit tricky to fail a pytest run, after all the tests have already passed. I know the cov plugin uses some hacky way to accomplish this, but I haven't had the time to figure out if I can match that behavior.

[adamcunnington-gdt]

@Daverball thanks for the response.

I think treating an empty string the same as unset is fine - and common practice (this would certainly be a super fast win please!).

Another alternative (if you don't wish to delay validation as you mention) is to allow me to provide some explicit option to actively supress uploading, e.g. --no-codecov. You can then hang behaviour off that.

[Daverball]

@adamcunnington-gdt The error is generated during the argument parsing, so there isn't really anything to disable at this point. It would not actually upload anything if the provided token were valid, since the actual upload step does require explicitly setting --codecov. It just complains about the invalidity of the token, since validation happens during argument parsing. It errors out before pytest does anything note worthy at all.

[adamcunnington-gdt]

Ah ok - are you happy to implement a small change that treats empty string as unset at least?

[Daverball]

Absolutely.

[Daverball]

Implemented treatment of empty environment variables via d9bcd28