Skip to content

Commit

Permalink
fix(log): keep log emitter running until the run managed (#290)
Browse files Browse the repository at this point in the history
The log emitter paused too early that not logging the command it runs
in the managed mode.
  • Loading branch information
syu-w authored Apr 1, 2024
1 parent 226fd10 commit 7e9ea22
Show file tree
Hide file tree
Showing 4 changed files with 16 additions and 14 deletions.
19 changes: 12 additions & 7 deletions craft_application/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -344,14 +344,19 @@ def run_managed(self, platform: str | None, build_for: str | None) -> None:
with self.services.provider.instance(
build_info, work_dir=self._work_dir
) as instance:
cmd = [self.app.name, *sys.argv[1:]]
craft_cli.emit.debug(
f"Executing {cmd} in instance location {instance_path} with {extra_args}."
)
try:
# Pyright doesn't fully understand craft_providers's CompletedProcess.
instance.execute_run( # pyright: ignore[reportUnknownMemberType,reportUnknownVariableType]
[self.app.name, *sys.argv[1:]],
cwd=instance_path,
check=True,
**extra_args,
)
with craft_cli.emit.pause():
# Pyright doesn't fully understand craft_providers's CompletedProcess.
instance.execute_run( # pyright: ignore[reportUnknownMemberType,reportUnknownVariableType]
cmd,
cwd=instance_path,
check=True,
**extra_args,
)
except subprocess.CalledProcessError as exc:
raise craft_providers.ProviderError(
f"Failed to execute {self.app.name} in instance."
Expand Down
3 changes: 1 addition & 2 deletions craft_application/services/provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -133,8 +133,7 @@ def instance(
emit.debug("Instance launched and working directory mounted")
self._setup_instance_bashrc(instance)
try:
with emit.pause():
yield instance
yield instance
finally:
self._capture_logs_from_instance(instance)

Expand Down
4 changes: 0 additions & 4 deletions tests/unit/services/test_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -290,15 +290,13 @@ def test_instance(
provider_service,
base_name,
allow_unstable,
mocker,
):
mock_provider = mock.MagicMock(spec=craft_providers.Provider)
monkeypatch.setattr(
provider_service,
"get_provider",
lambda name: mock_provider, # noqa: ARG005 (unused argument)
)
spy_pause = mocker.spy(provider.emit, "pause")
arch = util.get_host_architecture()
build_info = models.BuildInfo("foo", arch, arch, base_name)

Expand Down Expand Up @@ -326,8 +324,6 @@ def test_instance(
)
with check:
emitter.assert_progress("Launching managed .+ instance...", regex=True)
with check:
assert spy_pause.call_count == 1


def test_load_bashrc(emitter):
Expand Down
4 changes: 3 additions & 1 deletion tests/unit/test_application.py
Original file line number Diff line number Diff line change
Expand Up @@ -237,11 +237,12 @@ def test_log_path(monkeypatch, app, provider_managed, expected):
assert actual == expected


def test_run_managed_success(app, fake_project, fake_build_plan):
def test_run_managed_success(mocker, app, fake_project, fake_build_plan):
mock_provider = mock.MagicMock(spec_set=services.ProviderService)
app.services.provider = mock_provider
app.project = fake_project
app._build_plan = fake_build_plan
mock_pause = mocker.spy(craft_cli.emit, "pause")
arch = get_host_architecture()

app.run_managed(None, arch)
Expand All @@ -253,6 +254,7 @@ def test_run_managed_success(app, fake_project, fake_build_plan):
)
in mock_provider.instance.mock_calls
)
mock_pause.assert_called_once()


def test_run_managed_failure(app, fake_project, fake_build_plan):
Expand Down

0 comments on commit 7e9ea22

Please sign in to comment.