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

chore: unit tests around protocol usage #48

Merged
merged 4 commits into from
Jul 16, 2024
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions slack_cli_hooks/hooks/doctor.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,7 @@
import json
import platform

from slack_cli_hooks.protocol import (
Protocol,
build_protocol,
)
from slack_cli_hooks.protocol import Protocol, build_protocol

PROTOCOL: Protocol

Expand Down
10 changes: 3 additions & 7 deletions slack_cli_hooks/hooks/get_hooks.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
#!/usr/bin/env python
import json
import sys
from slack_cli_hooks.protocol import (
Protocol,
MessageBoundaryProtocol,
DefaultProtocol,
build_protocol,
)

from slack_cli_hooks.protocol import DefaultProtocol, MessageBoundaryProtocol, Protocol, build_protocol

PROTOCOL: Protocol

Expand All @@ -30,5 +26,5 @@
}

if __name__ == "__main__":
PROTOCOL = build_protocol()
PROTOCOL = build_protocol(argv=sys.argv[1:])
Copy link
Member

Choose a reason for hiding this comment

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

I'm for this argument being explicit instead of falling back to the same default, but I'm wondering if we'd also want to remove that defaulting value with this change? That might be a breaking change though, so feel free to ignore - this adds a bunch of clarity as is!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like this 💯 it isn't a breaking change since the build_protocol util is not exposed publicly

PROTOCOL.respond(json.dumps(hooks_payload))
3 changes: 2 additions & 1 deletion slack_cli_hooks/hooks/get_manifest.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#!/usr/bin/env python
import os
import re
import sys
from typing import List

from slack_cli_hooks.error import CliError
Expand Down Expand Up @@ -46,5 +47,5 @@ def get_manifest(working_directory: str) -> str:


if __name__ == "__main__":
PROTOCOL = build_protocol()
PROTOCOL = build_protocol(argv=sys.argv[1:])
PROTOCOL.respond(get_manifest(os.getcwd()))
2 changes: 1 addition & 1 deletion slack_cli_hooks/hooks/start.py
Original file line number Diff line number Diff line change
Expand Up @@ -40,5 +40,5 @@ def start(working_directory: str) -> None:


if __name__ == "__main__":
PROTOCOL = build_protocol()
PROTOCOL = build_protocol(argv=sys.argv[1:])
start(os.getcwd())
7 changes: 2 additions & 5 deletions slack_cli_hooks/protocol/__init__.py
Original file line number Diff line number Diff line change
@@ -1,15 +1,12 @@
import argparse
import sys
from typing import List

from .default_protocol import DefaultProtocol
from .message_boundary_protocol import MessageBoundaryProtocol
from .protocol import Protocol

__all__ = [
"DefaultProtocol",
"MessageBoundaryProtocol",
"Protocol",
]
__all__ = ["DefaultProtocol" "MessageBoundaryProtocol", "Protocol"]


def build_protocol(argv: List[str] = sys.argv[1:]) -> Protocol:
Expand Down
1 change: 1 addition & 0 deletions slack_cli_hooks/protocol/message_boundary_protocol.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging

from .protocol import Protocol


Expand Down
38 changes: 38 additions & 0 deletions tests/mock_protocol.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
from unittest.mock import Mock

from slack_cli_hooks.protocol.protocol import Protocol


def debug(self, msg: str, *args, **kwargs):
"""This is a mock"""
pass


def info(self, msg: str, *args, **kwargs):
"""This is a mock"""
pass


def warning(self, msg: str, *args, **kwargs):
"""This is a mock"""
pass


def error(self, msg: str, *args, **kwargs):
"""This is a mock"""
pass


def respond(self, data: str):
"""This is a mock"""
pass


class MockProtocol(Protocol):
name: str = "MockProtocol"

debug = Mock(spec=debug, return_value=None)
info = Mock(spec=info, return_value=None)
warning = Mock(spec=warning, return_value=None)
error = Mock(spec=error, return_value=None)
respond = Mock(spec=respond, return_value=None)
5 changes: 3 additions & 2 deletions tests/mock_socket_mode_server.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
import json
import threading
import time
from urllib.request import urlopen
from urllib.error import URLError
from unittest import TestCase
from urllib.error import URLError
from urllib.request import urlopen

from flask import Flask
from gevent import pywsgi
from geventwebsocket.handler import WebSocketHandler
Expand Down
2 changes: 1 addition & 1 deletion tests/mock_web_api_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from http.server import HTTPServer, SimpleHTTPRequestHandler
from typing import Type
from unittest import TestCase
from urllib.parse import urlparse, ParseResult
from urllib.parse import ParseResult, urlparse


class MockHandler(SimpleHTTPRequestHandler):
Expand Down
2 changes: 1 addition & 1 deletion tests/scenario_test/test_app/app.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import os

from slack_sdk import WebClient
from slack_bolt.app import App
from slack_sdk import WebClient
from utils import get_test_socket_mode_handler, wait_for_test_socket_connection

web_client = WebClient(base_url="http://localhost:8888", token=os.environ.get("SLACK_BOT_TOKEN"))
Expand Down
2 changes: 1 addition & 1 deletion tests/scenario_test/test_app/my_app.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import os

from slack_sdk import WebClient
from slack_bolt.app import App
from slack_sdk import WebClient
from utils import get_test_socket_mode_handler, wait_for_test_socket_connection

web_client = WebClient(base_url="http://localhost:8888", token=os.environ.get("SLACK_BOT_TOKEN"))
Expand Down
2 changes: 1 addition & 1 deletion tests/scenario_test/test_app/utils.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import time

from typing import Optional

from slack_bolt.adapter.socket_mode.builtin import SocketModeHandler
from slack_bolt.app.app import App

Expand Down
4 changes: 2 additions & 2 deletions tests/scenario_test/test_check_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@

from slack_cli_hooks.hooks import check_update
from slack_cli_hooks.hooks.check_update import build_output
from slack_cli_hooks.protocol.default_protocol import DefaultProtocol
from tests.mock_protocol import MockProtocol
from tests.utils import build_fake_dependency, build_fake_pypi_urlopen


class TestGetManifest:
def setup_method(self):
check_update.PROTOCOL = DefaultProtocol()
check_update.PROTOCOL = MockProtocol()

def test_build_output(self):
test_project = "test_proj"
Expand Down
13 changes: 12 additions & 1 deletion tests/scenario_test/test_get_hooks.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,23 @@
import json
import runpy
import sys
from unittest.mock import patch

from slack_cli_hooks.hooks import get_hooks, get_manifest, start


class TestGetHooks:

def setup_method(self):
cli_args = [get_hooks.__name__, "--protocol", "message-boundaries", "--boundary", ""]
self.argv_mock = patch.object(sys, "argv", cli_args)
self.argv_mock.start()

def teardown_method(self):
self.argv_mock.stop()

def test_get_manifest(self, capsys):
runpy.run_module(get_hooks.__name__, run_name="__main__")
runpy.run_module(get_hooks.__name__, run_name="__main__", alter_sys=False)
Copy link
Member

Choose a reason for hiding this comment

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

Does this change need to be applied to other runpy.run_module methods? It seems like False is the default value but it'd be nice to keep this consistent in whatever case IMO! 🙏

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch 💯 this is a remnant of my experimental work


out, err = capsys.readouterr()
json_response = json.loads(out)
Expand Down
9 changes: 9 additions & 0 deletions tests/scenario_test/test_get_manifest.py
Original file line number Diff line number Diff line change
@@ -1,17 +1,26 @@
import json
import os
import runpy
import sys
from unittest.mock import patch

import pytest

from slack_cli_hooks.error import CliError
from slack_cli_hooks.hooks import get_manifest


class TestGetManifest:

def setup_method(self):
cli_args = [get_manifest.__name__, "--protocol", "message-boundaries", "--boundary", ""]
self.argv_mock = patch.object(sys, "argv", cli_args)
self.argv_mock.start()
self.cwd = os.getcwd()

def teardown_method(self):
os.chdir(self.cwd)
self.argv_mock.stop()

def test_get_manifest_script(self, capsys):
working_directory = "tests/scenario_test/test_app"
Expand Down
13 changes: 11 additions & 2 deletions tests/scenario_test/test_start.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
import os
import runpy
import sys
from unittest.mock import patch

import pytest
import os
from slack_cli_hooks.error import CliError

from slack_cli_hooks.error import CliError
from slack_cli_hooks.hooks import start
from tests.mock_socket_mode_server import start_socket_mode_server, stop_socket_mode_server
from tests.mock_web_api_server import cleanup_mock_web_api_server, setup_mock_web_api_server
Expand All @@ -18,9 +21,15 @@ def setup_method(self):
os.environ["SLACK_APP_TOKEN"] = "xapp-A111-222-xyz"
setup_mock_web_api_server(self)
start_socket_mode_server(self, 3012)

cli_args = [start.__name__, "--protocol", "message-boundaries", "--boundary", ""]
self.argv_mock = patch.object(sys, "argv", cli_args)
self.argv_mock.start()

self.cwd = os.getcwd()

def teardown_method(self):
self.argv_mock.stop()
os.chdir(self.cwd)
os.environ.pop("SLACK_BOT_TOKEN", None)
os.environ.pop("SLACK_APP_TOKEN", None)
Expand Down
8 changes: 4 additions & 4 deletions tests/slack_cli_hooks/hooks/test_check_update.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
from urllib import request

import pytest

from packaging.version import Version

from slack_cli_hooks.error import PypiError
from slack_cli_hooks.hooks import check_update
from slack_cli_hooks.hooks.check_update import (
Expand All @@ -14,15 +14,15 @@
pypi_get,
pypi_get_json,
)
from slack_cli_hooks.protocol.default_protocol import DefaultProtocol
from tests.mock_protocol import MockProtocol
from tests.utils import build_fake_dependency, build_fake_pypi_urlopen


class TestRelease:
test_project = "test_proj"

def setup_method(self):
check_update.PROTOCOL = DefaultProtocol()
check_update.PROTOCOL = MockProtocol()

def test_release_with_same_version(self):
release = Release(name=self.test_project, current=Version("0.0.0"), latest=Version("0.0.0"))
Expand Down Expand Up @@ -57,7 +57,7 @@ def test_release_with_major_upgrade(self):

class TestCheckUpdate:
def setup_method(self):
check_update.PROTOCOL = DefaultProtocol()
check_update.PROTOCOL = MockProtocol()

def test_pypi_get(self):
test_project = "test_proj"
Expand Down
1 change: 1 addition & 0 deletions tests/slack_cli_hooks/hooks/test_get_hooks.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import re

from slack_cli_hooks.hooks.get_hooks import hooks_payload


Expand Down
1 change: 1 addition & 0 deletions tests/slack_cli_hooks/hooks/test_get_manifest.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from unittest import mock

import pytest

from slack_cli_hooks.error import CliError
from slack_cli_hooks.hooks.get_manifest import filter_directories, find_file_path

Expand Down
3 changes: 2 additions & 1 deletion tests/slack_cli_hooks/protocol/test_protocol_factory.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from slack_cli_hooks.protocol import build_protocol, DefaultProtocol, MessageBoundaryProtocol, Protocol
from slack_cli_hooks.protocol import MessageBoundaryProtocol, Protocol, build_protocol
from slack_cli_hooks.protocol.default_protocol import DefaultProtocol


class TestProtocolFactory:
Expand Down