Temporary work-around to #793 by turning off strict argument checking

[hildjj]

Marked all failing tests as 'pending'. Put TODOs on each change, so that when the upstream bug is resolved, this can be backed out easily.

Note to maintainers: this is a suggestion with my developer hat on. Please treat this as a normal request from an external user, with no special authority.

[coveralls]

Coverage remained the same at 100.0% when pulling e181222 on hildjj:master into 8844e50 on mozilla:master.

[coveralls]

Coverage remained the same at 100.0% when pulling e181222 on hildjj:master into 8844e50 on mozilla:master.

[rpl]

It would be great if we could avoid disabling strict (especially given that the upstream bug is also open for a long time, I'm not sure that a fix will come from a new yargs version anytime soon).

Another option could be to check if we can clear the environment variables that triggers the yargs failure before yargs did the parsing.

If we want to fix the issue with the credential environment variables in particular, that may be doable and not too complex.

[rpl]

(As a side note, the failure on the travis job is not related to test failures, the relevant ones are skipped in the same patch, but to the commit message, because it doesn't currently follow the commitlint conventions used in this repository)

[Rob--W]

It would be great if we could avoid disabling strict (especially given that the upstream bug is also open for a long time, I'm not sure that a fix will come from a new yargs version anytime soon).

Disabling strict checking would resolve #793, at the cost of losing the ability to provide feedback about typos or other unrecognized flags. The cost outweighs the benefit in my opinion.

Another option could be to check if we can clear the environment variables that triggers the yargs failure before yargs did the parsing.

I was also thinking of this. It may be possible to create a general implementation that iterates over all registered flags and then stripping the environment variables (and maybe even warn about the ignored ones), but if specific examples are provided, we can try to look into specific, less general ways to address the issue at hand.

@hildjj Which environment variables encouraged you to create this PR?

[hildjj]

I have WEB_EXT_FIREFOX_PROFILE, WEB_EXT_FIREFOX, and WEB_EXT_API_KEY set in the environment I'm using. It means I can't use either web-ext run or web-ext sign.

[hildjj]

I was about to suggest that another work-around would be to finish #176 including docs, but that approach also does not work with strict on for the same reason.

I am not really convinced by the argument that having basic functionality work is less important than better first-time user experience in the face of typos. However, I'll repeat that it's not my call and I'll defer to folks who have dealt with this community for years.

[rpl]

I just wanted to double-check how complex was going to be to implement the strategy I was suggesting, and given that it turned out to be a pretty small change I pushed it in #1812 so that we can take a look at it and agree if it is a reasonable approach as I think.

(and I also just added a unit test and an integration test to the PR, and so if the strategy looks a compelling one the pull request is also ready for review).

[rpl]

Closing as superseded by #1812 (and has been just merged on master).