Fix CLI tests after Click 7.1 release #90
Conversation
| u'\nTry "delphix-sdk build -h" for help.' | ||
| u'\nTry \'delphix-sdk build -h\' for help.' | ||
| u'\n' | ||
| u'\nError: Invalid value for "-c" /' | ||
| u' "--plugin-config": File' | ||
| u' "/not/a/real/file/plugin_config.yml"' | ||
| u'\nError: Invalid value for \'-c\' /' | ||
| u' \'--plugin-config\': File' | ||
| u' \'/not/a/real/file/plugin_config.yml\'' |
There was a problem hiding this comment.
nit: I think our standard is if you're using single quotes in the string, we can then use double quotes to start the string (so that we don't have to use backslash):
so it would be something like:
assert result.output == (u"Usage: delphix-sdk build [OPTIONS]"
u"\nTry 'delphix-sdk build -h' for help."
u"\n"
u"\nError: Invalid value for '-c' /"
u"'--plugin-config': File"
u" '/not/a/real/file/plugin_config.yml'"
| assert result.output == (u'Usage: delphix-sdk upload [OPTIONS]' | ||
| u'\nTry "delphix-sdk upload -h" for help.' | ||
| u'\nTry \'delphix-sdk upload -h\' for help.' | ||
| u'\n' | ||
| u'\nError: Invalid value for "-a" /' | ||
| u' "--upload-artifact": File' | ||
| u' "/not/a/real/file/artifact.json"' | ||
| u'\nError: Invalid value for \'-a\' /' | ||
| u' \'--upload-artifact\': File' | ||
| u' \'/not/a/real/file/artifact.json\'' | ||
| u' does not exist.' | ||
| u'\n') |
There was a problem hiding this comment.
That's what I was going to do but then I would have to change the entire test_cli.py to be consistent. I will have to check what convention we followed in other files.
There was a problem hiding this comment.
I just read the following recommendation from PEP8 (https://www.python.org/dev/peps/pep-0008/):
In Python, single-quoted strings and double-quoted strings are the same. This PEP does not make a recommendation for this. Pick a rule and stick to it. When a string contains single or double quote characters, however, use the other one to avoid backslashes in the string. It improves readability.
I will get rid of the backslashes.
ravi-cm
left a comment
ravi-cm
left a comment
There was a problem hiding this comment.
Looks good. Please run a CLI dvp to make sure o/p is fine for the help option or if a an argument is missed.
ankursarin
left a comment
ankursarin
left a comment
There was a problem hiding this comment.
I think we also need to change tools/setup.py to update the version of click to be
"click >= 7.1"?
5 participants
Pull request checklist
Please check if your PR fulfills the following requirements:
Pull request type
Please check the type of change your PR introduces:
What is the current behavior?
After Click 7.1 release (March 9, 2020), the CLI tests started failing with errors of this sort:
It looks like the click CLI now outputs single rather than double quotes in its error messages. This is related to the following commit that did manual cleanup before the release. The commit has been shipped in versions 7.1 and 7.1.1.
What is the new behavior?
Modify CLI tests to expect single rather than double quotes.
Does this introduce a breaking change?
Other information