Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

build-providers: add support for --shell-after #2253

Merged
merged 4 commits into from Sep 10, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 3 additions & 1 deletion snapcraft/cli/_options.py
Expand Up @@ -31,16 +31,18 @@ def get_help_record(self, ctx):
"--no-parallel-builds",
"--target-arch",
"--debug",
"--shell-after",
]

_BUILD_OPTIONS = [
dict(
is_flag=True,
help=("Detect best candidate location for stage-packages using geoip"),
help="Detect best candidate location for stage-packages using geoip",
),
dict(is_flag=True, help="Force a sequential build."),
dict(metavar="<arch>", help="Target architecture to cross compile to"),
dict(is_flag=True, help="Shells into the environment if the build fails."),
dict(is_flag=True, help="Shells into the environment after the step has run."),
]


Expand Down
3 changes: 3 additions & 0 deletions snapcraft/cli/lifecycle.py
Expand Up @@ -45,6 +45,7 @@ def _execute(
parts: str,
pack_project: bool = False,
output: str = None,
shell_after: bool = False,
**kwargs
) -> "Project":
# fmt: on
Expand All @@ -65,6 +66,8 @@ def _execute(
instance.pack_project(output=output)
else:
instance.execute_step(step)
if shell_after:
instance.shell()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a little surprising. What happens if the shell exits with error status, at this point?
I'd expect the shell_after check to be done after we'd cleared the try/except block -- if necessary with its own exception handler.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it will raise a proper exception that is captured correctly in our exit handler

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is that what you want? the step itself didn't fail, just the shell after. Are the same messages still applicable? Do you want shell and debug to interact in this way?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point now. Will fix too

except Exception as exc:
if project.debug:
instance.shell()
Expand Down
59 changes: 59 additions & 0 deletions tests/unit/commands/test_build_providers.py
Expand Up @@ -15,6 +15,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.

from unittest import mock
from typing import Optional

import fixtures
from testtools.matchers import Equals
Expand Down Expand Up @@ -69,3 +70,61 @@ def test_step_without_debug_using_build_provider_fails_and_does_not_shell(self):
self.assertRaises(ProviderExecError, self.run_command, ["pull"])

self.shell_mock.assert_not_called()


class BuildProviderShellCommandTestCase(LifecycleCommandsBaseTestCase):
def setUp(self):
super().setUp()
self.useFixture(
fixtures.EnvironmentVariable("SNAPCRAFT_BUILD_ENVIRONMENT", "multipass")
)

shell_mock = mock.Mock()
pack_project_mock = mock.Mock()
execute_step_mock = mock.Mock()

class Provider(ProviderImpl):
def pack_project(self, *, output: Optional[str] = None) -> None:
pack_project_mock(output)

def execute_step(self, step: steps.Step) -> None:
execute_step_mock(step)

def shell(self):
shell_mock()

patcher = mock.patch(
"snapcraft.internal.build_providers.get_provider_for", return_value=Provider
)
self.provider = patcher.start()
self.addCleanup(patcher.stop)

self.shell_mock = shell_mock
self.pack_project_mock = pack_project_mock
self.execute_step_mock = execute_step_mock

self.make_snapcraft_yaml("pull", base="core18")

def test_step_with_shell_after(self):
result = self.run_command(["pull", "--shell-after"])

self.assertThat(result.exit_code, Equals(0))
self.pack_project_mock.assert_not_called()
self.execute_step_mock.assert_called_once_with(steps.PULL)
self.shell_mock.assert_called_once_with()

def test_snap_with_shell_after(self):
result = self.run_command(["snap", "--output", "fake.snap", "--shell-after"])

self.assertThat(result.exit_code, Equals(0))
self.pack_project_mock.assert_called_once_with("fake.snap")
self.execute_step_mock.assert_not_called()
self.shell_mock.assert_called_once_with()

def test_step_without_shell_after_does_not_enter_shell(self):
result = self.run_command(["pull"])

self.assertThat(result.exit_code, Equals(0))
self.pack_project_mock.assert_not_called()
self.execute_step_mock.assert_called_once_with(steps.PULL)
self.shell_mock.assert_not_called()