Skip to content

Commit

Permalink
codereview: rename, remove useless comments, improve prints
Browse files Browse the repository at this point in the history
  • Loading branch information
chouetz committed Feb 5, 2025
1 parent 6250ee6 commit c40480b
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 14 deletions.
8 changes: 4 additions & 4 deletions tasks/libs/common/check_tools_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,8 @@ def check_tools_installed(tools: list) -> bool:
"""
Check if the tools are installed in the system.
"""
for tool in tools:
if not shutil.which(tool):
print(f"{tool} is not installed. Please install it before running this task.")
return False
not_installed = [tool for tool in tools if not shutil.which(tool)]
if not_installed:
print(f"{color_message('ERROR', Color.RED)}: The following tools are not installed: {', '.join(not_installed)}")
return False
return True
7 changes: 3 additions & 4 deletions tasks/protobuf.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def generate(ctx, do_mockgen=True):
We must build the packages one at a time due to protoc-gen-go limitations
"""
# Key: path, Value: grpc_gateway, inject_tags
check_dependencies(ctx)
check_tools(ctx)
base = os.path.dirname(os.path.abspath(__file__))
repo_root = os.path.abspath(os.path.join(base, ".."))
proto_root = os.path.join(repo_root, "pkg", "proto")
Expand Down Expand Up @@ -151,7 +151,6 @@ def generate(ctx, do_mockgen=True):

# Check the generated files were properly committed
git_status = ctx.run("git status -suno", hide=True).stdout
# git_status = ctx.run("git status -suno | grep -E 'pkg/proto/pbgo.*.pb(.gw)?.go'").stdout.strip()
proto_file = re.compile(r"pkg/proto/pbgo/.*\.pb(\.gw)?\.go$")
if any(proto_file.search(line) for line in git_status.split("\n")):
raise Exit(
Expand All @@ -160,7 +159,7 @@ def generate(ctx, do_mockgen=True):
)


def check_dependencies(ctx):
def check_tools(ctx):
"""
Check if all the required dependencies are installed
"""
Expand All @@ -175,6 +174,6 @@ def check_dependencies(ctx):
raise Exit(
f"Expected protoc version {expected_version}, found {current_version}. Please run `inv install-protoc` before running this task.",
code=1,
) from None
)
except UnexpectedExit as e:
raise Exit("protoc is not installed. Please install it before running this task.", code=1) from e
11 changes: 5 additions & 6 deletions tasks/unit_tests/libs/common/check_tools_tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from invoke import Context, Exit, MockContext, Result, UnexpectedExit

from tasks.libs.common.check_tools_version import check_tools_installed
from tasks.protobuf import check_dependencies
from tasks.protobuf import check_tools


class TestCheckToolsInstalled(unittest.TestCase):
Expand All @@ -18,13 +18,12 @@ def test_tool_not_installed(self):
self.assertFalse(check_tools_installed(["not_installed_tool", "ls"]))


class TestCheckDependencies(unittest.TestCase):
class TestCheckTools(unittest.TestCase):
@patch('tasks.protobuf.check_tools_installed', new=MagicMock(return_value=False))
def test_tools_not_installed(self):
c = Context()
with self.assertRaises(Exit) as e:
check_dependencies(c)
print(e)
check_tools(c)
self.assertEqual(
e.exception.message, "Please install the required tools with `inv install-tools` before running this task."
)
Expand All @@ -33,13 +32,13 @@ def test_tools_not_installed(self):
def test_bad_protoc(self):
c = MockContext(run={'protoc --version': Result("libprotoc 1.98.2")})
with self.assertRaises(Exit) as e:
check_dependencies(c)
check_tools(c)
self.assertTrue(e.exception.message.startswith("Expected protoc version 29.3, found"))

@patch('tasks.protobuf.check_tools_installed', new=MagicMock(return_value=True))
def test_protoc_not_installed(self):
c = MagicMock()
c.run.side_effect = UnexpectedExit("protoc --version")
with self.assertRaises(Exit) as e:
check_dependencies(c)
check_tools(c)
self.assertEqual(e.exception.message, "protoc is not installed. Please install it before running this task.")

0 comments on commit c40480b

Please sign in to comment.