Skip to content

Commit

Permalink
ref(commands): Improving error messages (#195)
Browse files Browse the repository at this point in the history
* ref(commands): Improving error messages

* Adding test coverage
  • Loading branch information
IanWoodard authored Jan 2, 2025
1 parent e40aa4a commit e06c09d
Show file tree
Hide file tree
Showing 12 changed files with 152 additions and 6 deletions.
7 changes: 7 additions & 0 deletions devservices/commands/down.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY
from devservices.constants import DEVSERVICES_DIR_NAME
from devservices.exceptions import ConfigError
from devservices.exceptions import ConfigNotFoundError
from devservices.exceptions import DependencyError
from devservices.exceptions import DockerComposeError
from devservices.exceptions import ServiceNotFoundError
Expand Down Expand Up @@ -56,6 +57,12 @@ def down(args: Namespace) -> None:
service_name = args.service_name
try:
service = find_matching_service(service_name)
except ConfigNotFoundError as e:
capture_exception(e)
console.failure(
f"{str(e)}. Please specify a service (i.e. `devservices down sentry`) or run the command from a directory with a devservices configuration."
)
exit(1)
except ConfigError as e:
capture_exception(e)
console.failure(str(e))
Expand Down
7 changes: 7 additions & 0 deletions devservices/commands/list_dependencies.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from sentry_sdk import capture_exception

from devservices.exceptions import ConfigError
from devservices.exceptions import ConfigNotFoundError
from devservices.exceptions import ServiceNotFoundError
from devservices.utils.console import Console
from devservices.utils.services import find_matching_service
Expand All @@ -32,6 +33,12 @@ def list_dependencies(args: Namespace) -> None:

try:
service = find_matching_service(service_name)
except ConfigNotFoundError as e:
capture_exception(e)
console.failure(
f"{str(e)}. Please specify a service (i.e. `devservices list-dependencies sentry`) or run the command from a directory with a devservices configuration."
)
exit(1)
except ConfigError as e:
capture_exception(e)
console.failure(str(e))
Expand Down
7 changes: 7 additions & 0 deletions devservices/commands/logs.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from devservices.constants import DEVSERVICES_DIR_NAME
from devservices.constants import MAX_LOG_LINES
from devservices.exceptions import ConfigError
from devservices.exceptions import ConfigNotFoundError
from devservices.exceptions import DependencyError
from devservices.exceptions import DockerComposeError
from devservices.exceptions import ServiceNotFoundError
Expand Down Expand Up @@ -46,6 +47,12 @@ def logs(args: Namespace) -> None:
service_name = args.service_name
try:
service = find_matching_service(service_name)
except ConfigNotFoundError as e:
capture_exception(e)
console.failure(
f"{str(e)}. Please specify a service (i.e. `devservices logs sentry`) or run the command from a directory with a devservices configuration."
)
exit(1)
except ConfigError as e:
capture_exception(e)
console.failure(str(e))
Expand Down
7 changes: 7 additions & 0 deletions devservices/commands/status.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY
from devservices.constants import DEVSERVICES_DIR_NAME
from devservices.exceptions import ConfigError
from devservices.exceptions import ConfigNotFoundError
from devservices.exceptions import DependencyError
from devservices.exceptions import DockerComposeError
from devservices.exceptions import ServiceNotFoundError
Expand Down Expand Up @@ -86,6 +87,12 @@ def status(args: Namespace) -> None:
service_name = args.service_name
try:
service = find_matching_service(service_name)
except ConfigNotFoundError as e:
capture_exception(e)
console.failure(
f"{str(e)}. Please specify a service (i.e. `devservices status sentry`) or run the command from a directory with a devservices configuration."
)
exit(1)
except ConfigError as e:
capture_exception(e)
console.failure(str(e))
Expand Down
7 changes: 7 additions & 0 deletions devservices/commands/up.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from devservices.constants import DEVSERVICES_DEPENDENCIES_CACHE_DIR_KEY
from devservices.constants import DEVSERVICES_DIR_NAME
from devservices.exceptions import ConfigError
from devservices.exceptions import ConfigNotFoundError
from devservices.exceptions import ContainerHealthcheckFailedError
from devservices.exceptions import DependencyError
from devservices.exceptions import DockerComposeError
Expand Down Expand Up @@ -60,6 +61,12 @@ def up(args: Namespace) -> None:
service_name = args.service_name
try:
service = find_matching_service(service_name)
except ConfigNotFoundError as e:
capture_exception(e)
console.failure(
f"{str(e)}. Please specify a service (i.e. `devservices up sentry`) or run the command from a directory with a devservices configuration."
)
exit(1)
except ConfigError as e:
capture_exception(e)
console.failure(str(e))
Expand Down
4 changes: 3 additions & 1 deletion devservices/configs/service_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,9 @@ def _validate(self) -> None:
def load_service_config_from_file(repo_path: str) -> ServiceConfig:
config_path = os.path.join(repo_path, DEVSERVICES_DIR_NAME, CONFIG_FILE_NAME)
if not os.path.exists(config_path):
raise ConfigNotFoundError(f"Config file not found in directory: {config_path}")
raise ConfigNotFoundError(
f"No devservices configuration found in {config_path}"
)
with open(config_path, "r", encoding="utf-8") as stream:
try:
config = yaml.safe_load(stream)
Expand Down
25 changes: 24 additions & 1 deletion tests/commands/test_down.py
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,30 @@ def test_down_simple(
assert "Stopping redis" in captured.out.strip()


@mock.patch("devservices.utils.state.State.remove_started_service")
def test_down_no_config_file(
mock_remove_started_service: mock.Mock,
capsys: pytest.CaptureFixture[str],
tmp_path: Path,
) -> None:
os.chdir(tmp_path)

args = Namespace(service_name=None, debug=False)

with pytest.raises(SystemExit):
down(args)

# Capture the printed output
captured = capsys.readouterr()

assert (
f"No devservices configuration found in {tmp_path}/devservices/config.yml. Please specify a service (i.e. `devservices down sentry`) or run the command from a directory with a devservices configuration."
in captured.out.strip()
)

mock_remove_started_service.assert_not_called()


@mock.patch("devservices.utils.docker_compose.subprocess.run")
@mock.patch("devservices.utils.state.State.remove_started_service")
def test_down_error(
Expand Down Expand Up @@ -150,7 +174,6 @@ def test_down_error(

mock_remove_started_service.assert_not_called()

captured = capsys.readouterr()
assert "Stopping clickhouse" not in captured.out.strip()
assert "Stopping redis" not in captured.out.strip()

Expand Down
21 changes: 21 additions & 0 deletions tests/commands/test_list_dependencies.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import os
from argparse import Namespace
from pathlib import Path
from unittest import mock
Expand All @@ -14,6 +15,26 @@
from devservices.utils.services import Service


def test_list_dependencies_no_config_file(
capsys: pytest.CaptureFixture[str],
tmp_path: Path,
) -> None:
os.chdir(tmp_path)

args = Namespace(service_name=None, debug=False)

with pytest.raises(SystemExit):
list_dependencies(args)

# Capture the printed output
captured = capsys.readouterr()

assert (
f"No devservices configuration found in {tmp_path}/devservices/config.yml. Please specify a service (i.e. `devservices list-dependencies sentry`) or run the command from a directory with a devservices configuration."
in captured.out.strip()
)


@mock.patch("devservices.commands.list_dependencies.find_matching_service")
def test_list_dependencies_service_not_found(
mock_find_matching_service: mock.Mock,
Expand Down
21 changes: 21 additions & 0 deletions tests/commands/test_logs.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import os
import subprocess
from argparse import Namespace
from pathlib import Path
Expand Down Expand Up @@ -152,6 +153,26 @@ def test_logs_no_specified_service_success(
assert captured.out.endswith("redis and clickhouse log output\n")


def test_logs_no_config_file(
capsys: pytest.CaptureFixture[str],
tmp_path: Path,
) -> None:
os.chdir(tmp_path)

args = Namespace(service_name=None, debug=False)

with pytest.raises(SystemExit):
logs(args)

# Capture the printed output
captured = capsys.readouterr()

assert (
f"No devservices configuration found in {tmp_path}/devservices/config.yml. Please specify a service (i.e. `devservices logs sentry`) or run the command from a directory with a devservices configuration."
in captured.out.strip()
)


@mock.patch("devservices.commands.logs.find_matching_service")
def test_logs_config_error(
find_matching_service_mock: mock.Mock,
Expand Down
21 changes: 21 additions & 0 deletions tests/commands/test_status.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import os
import subprocess
from argparse import Namespace
from pathlib import Path
Expand All @@ -15,6 +16,26 @@
from devservices.utils.services import Service


def test_status_no_config_file(
capsys: pytest.CaptureFixture[str],
tmp_path: Path,
) -> None:
os.chdir(tmp_path)

args = Namespace(service_name=None, debug=False)

with pytest.raises(SystemExit):
status(args)

# Capture the printed output
captured = capsys.readouterr()

assert (
f"No devservices configuration found in {tmp_path}/devservices/config.yml. Please specify a service (i.e. `devservices status sentry`) or run the command from a directory with a devservices configuration."
in captured.out.strip()
)


@mock.patch("devservices.commands.status._status")
@mock.patch("devservices.commands.status.find_matching_service")
@mock.patch("devservices.commands.status.install_and_verify_dependencies")
Expand Down
29 changes: 26 additions & 3 deletions tests/commands/test_up.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,30 @@ def test_up_dependency_error(
assert "Starting redis" not in captured.out.strip()


@mock.patch("devservices.utils.state.State.update_started_service")
def test_up_no_config_file(
mock_update_started_service: mock.Mock,
capsys: pytest.CaptureFixture[str],
tmp_path: Path,
) -> None:
os.chdir(tmp_path)

args = Namespace(service_name=None, debug=False)

with pytest.raises(SystemExit):
up(args)

# Capture the printed output
captured = capsys.readouterr()

assert (
f"No devservices configuration found in {tmp_path}/devservices/config.yml. Please specify a service (i.e. `devservices up sentry`) or run the command from a directory with a devservices configuration."
in captured.out.strip()
)

mock_update_started_service.assert_not_called()


@mock.patch("devservices.utils.docker_compose.subprocess.run")
@mock.patch("devservices.utils.state.State.update_started_service")
@mock.patch("devservices.commands.up._create_devservices_network")
Expand Down Expand Up @@ -249,9 +273,8 @@ def test_up_error(
env=mock.ANY,
)

captured = capsys.readouterr()
assert "Retrieving dependencies" not in captured.out.strip()
assert "Starting 'example-service' in mode: 'default'" not in captured.out.strip()
assert "Retrieving dependencies" in captured.out.strip()
assert "Starting 'example-service' in mode: 'default'" in captured.out.strip()
assert "Starting clickhouse" not in captured.out.strip()
assert "Starting redis" not in captured.out.strip()

Expand Down
2 changes: 1 addition & 1 deletion tests/configs/test_service_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ def test_load_service_config_from_file_missing_config(tmp_path: Path) -> None:
load_service_config_from_file(str(tmp_path))
assert (
str(e.value)
== f"Config file not found in directory: {tmp_path / 'devservices' / 'config.yml'}"
== f"No devservices configuration found in {tmp_path / 'devservices' / 'config.yml'}"
)


Expand Down

0 comments on commit e06c09d

Please sign in to comment.