file_utils: get_tool_path to always return an absolute path #2193
Conversation
sergiusens
force-pushed
the
get-tool-path
branch
2 times, most recently
from
8708b9a
to
1372f4a
Compare
Change get_tool_path to always return an absolute path and fail if the required command cannot be found. The unit tests have been adapted to not be path dependent and tests for retrieving environment dependent paths have been added. LP: #1785320 Signed-off-by: Sergio Schvezov <sergio.schvezov@canonical.com>
sergiusens
force-pushed
the
get-tool-path
branch
from
1372f4a
to
c4d7d7d
Compare
Codecov Report
@@ Coverage Diff @@
## master #2193 +/- ##
==========================================
- Coverage 91.22% 91.17% -0.05%
==========================================
Files 201 201
Lines 12909 12912 +3
Branches 1913 1913
==========================================
- Hits 11776 11773 -3
- Misses 770 772 +2
- Partials 363 367 +4
Continue to review full report at Codecov.
|
snapcraft/internal/errors.py
Outdated
|
|
||
| fmt = ( | ||
| "A tool snapcraft depends on could not be found: {command_name!r}.\n" | ||
| "Ensure the tool is installed and available, and try again." |
There was a problem hiding this comment.
This is a workaround, but in reality, for all the use-cases of this function so far (e.g. xdelta3, mksquashfs, etc.) all of these tools should ALWAYS be found, and if they're not, it's a bug we should know about. Worth adding "please log a bug" to this?
| @@ -79,64 +77,6 @@ def test_push_a_snap(self): | |||
| ) | |||
| mock_upload.assert_called_once_with("basic", self.snap_file) | |||
|
|
|||
| def test_push_a_snap_running_from_snap(self): | |||
There was a problem hiding this comment.
This test seems useful, is it duplicated somewhere? This comment applies to all the tests being removed, here. There has been a lot of breakage here historically.
There was a problem hiding this comment.
We clarified this offline: the tests covering get_tool_path here cover most of what is removed here. We need a suite that runs in docker to cover everything, which we can introduce later.
|
That last change enables this -> https://asciinema.org/a/hGSsKKhmPSPA92l8LyVDBAiBg |
|
spread 17.10 tests failed, ignoring those. |
Change get_tool_path to always return an absolute path and fail if
the required command cannot be found.
The unit tests have been adapted to not be path dependent and tests
for retrieving environment dependent paths have been added.
LP: #1785320
Signed-off-by: Sergio Schvezov sergio.schvezov@canonical.com
./runtests.sh static?./runtests.sh unit?