Join GitHub today
GitHub is home to over 20 million developers working together to host and review code, manage projects, and build software together.
Clear error when specifying a non existing command #350
Conversation
kyrofa
reviewed
Feb 29, 2016
| @@ -549,3 +549,13 @@ def test_exe_is_in_path(self, run_mock): | ||
| wrapper_contents = wrapper_file.read() | ||
| self.assertEqual(expected, wrapper_contents) | ||
| + | ||
| + def test_command_does_not_exist(self): |
kyrofa
Feb 29, 2016
Member
Can you add a test for a file that exists but is simply not executable as well?
|
I'd like one more test in there, but other than that this looks great |
elopio
reviewed
Feb 29, 2016
| @@ -222,10 +218,28 @@ def _wrap_exe(command, basename=None): | ||
| return os.path.relpath(wrappath, snap_dir) | ||
| +def _find_bin(binary, basedir): | ||
| + # If it doesn't exist it might be in the path | ||
| + logger.debug('Checking to see if {!r} is in the $PATH'.format(binary)) |
|
@kyrofa done |
|
Looks perfect! |
|
@kyrofa just for the kicks I've added a sunny day scenario as well |
elopio
reviewed
Feb 29, 2016
| + except CommandError as e: | ||
| + raise EnvironmentError( | ||
| + 'The specified command {!r} defined in {!r} does ' | ||
| + 'not exist'.format(str(e), app)) |
kyrofa
Feb 29, 2016
Member
I guess that's a good question. Are the exit codes 126 and 127? We can probably print the right message here depending.
sergiusens
Feb 29, 2016
Collaborator
If it is not executable it inherently does not exist from a "command" PoV.
sergiusens
Feb 29, 2016
Collaborator
From the manpage
1 if one or more specified commands is nonexistent or not exe-
cutable
I'll make the change, too bad since travis already ran
To be fair, people using the copy plugin get what they deserve! This would not happen when using a proper build system
kyrofa
reviewed
Feb 29, 2016
| + "defined in 'app1' does not exist", | ||
| + str(raised.exception)) | ||
| + | ||
| + def test_command_found(self): |
added a commit
that referenced
this pull request
Mar 1, 2016
sergiusens
merged commit 2886cc4
into
snapcore:master
Mar 1, 2016
sergiusens
deleted the
sergiusens:bugfix/1550496/exe-error
branch
Mar 1, 2016
pushed a commit
to smoser/snapcraft
that referenced
this pull request
Sep 14, 2016
pushed a commit
to kalikiana/snapcraft
that referenced
this pull request
Apr 6, 2017
sergiusens commentedFeb 29, 2016
When a command is supposed to be in PATH, make sure
the error is clear.
LP: #1550496
Signed-off-by: Sergio Schvezov sergio.schvezov@ubuntu.com