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

Sanitize MD and initiate addition of tests #43

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
27 changes: 27 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
name: Unit Test
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
name: Unit Test
name: Test

Copy link
Member Author

Choose a reason for hiding this comment

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

Made it such since we also have Smoke Test. Makes it more explicit on what is the difference between the two.


on:
- push
- pull_request

jobs:
typing:
runs-on: ubuntu-latest
steps:
- name: Check out repository
uses: actions/checkout@v4
with:
fetch-depth: 0
yarikoptic marked this conversation as resolved.
Show resolved Hide resolved

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.10'

- name: Install dependencies
run: |
python -m pip install --upgrade pip
python -m pip install --upgrade tox

- name: Run type checker
yarikoptic marked this conversation as resolved.
Show resolved Hide resolved
run: tox -e py3
5 changes: 5 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,11 @@ install_requires =
PyGithub ~= 2.0
ruamel.yaml ~= 0.15

[options.extras_require]
test =
pytest
pytest-cov
Copy link
Member

Choose a reason for hiding this comment

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

This isn't used by the test environment.

Copy link
Member

Choose a reason for hiding this comment

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

@yarikoptic pytest-cov is still unused.

Also, I would prefer it if, rather than defining a test extra, the test dependencies were instead listed in tox.ini.

Copy link
Member Author

Choose a reason for hiding this comment

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

why (what advantage) to list in a tool specific tox.ini making it harder to setup environment for any other tool/direct invocation of pytest? I would really prefer to concentrate dependencies in a single location.

Copy link
Member

Choose a reason for hiding this comment

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

I generally do not support any other tools than tox nor directly invoking pytest.

Copy link
Member Author

Choose a reason for hiding this comment

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

I typically invoke pytest directly, and testing within debian and most often conda packages is done directly via pytest. it is odd to require some additional utility whenever pytest framework is already sufficient.


[options.packages.find]
where = src

Expand Down
12 changes: 9 additions & 3 deletions src/solidation/__main__.py
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,7 @@ def to_markdown(self) -> str:
if user is None:
user = i.user.login
s += (
f"- [{i.title}]({i.html_url}) by {user}"
f"- [{sanitize_md(i.title)}]({i.html_url}) by {user}"
yarikoptic marked this conversation as resolved.
Show resolved Hide resolved
f" [{i.repository.full_name}]\n"
)

Expand All @@ -271,7 +271,8 @@ def to_markdown(self) -> str:
if untriaged_issues:
s += f"##### Untriaged issues of the last {dayscovered} days\n"
for i in sorted(untriaged_issues, key=lambda x: x.created_at):
s += f"- [{i.title}]({i.html_url}) [{i.repository.full_name}]\n"
s += f"- [{sanitize_md(i.title)}]({i.html_url}) " + \
f"[{i.repository.full_name}]\n"

s += (
f"##### Max {self.config.num_oldest_prs} oldest, open, non-draft"
Expand All @@ -297,7 +298,7 @@ def to_markdown(self) -> str:
# issue, which slows things down considerably.
continue
age = now - ensure_aware(i.created_at) # type: ignore[unreachable]
s += f"- [{i.title}]({i.html_url}) ({age.days} days old)\n"
s += f"- [{sanitize_md(i.title)}]({i.html_url}) ({age.days} days old)\n"
n_random_ip -= 1
if n_random_ip == 0:
break
Expand Down Expand Up @@ -437,6 +438,11 @@ def ensure_aware(dt: datetime) -> datetime:
return dt.replace(tzinfo=timezone.utc) if dt.tzinfo is None else dt


def sanitize_md(s: str) -> str:
# Remove `[]` symbols to ensure correct markdown in the references
return re.sub(r'([\\\[\]])', r'\\\1', s)
yarikoptic marked this conversation as resolved.
Show resolved Hide resolved


@click.command()
@click.option(
"-c",
Expand Down
Empty file added tests/__init__.py
Copy link
Member

Choose a reason for hiding this comment

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

This file is unnecessary and should be removed.

Empty file.
6 changes: 6 additions & 0 deletions tests/test_main.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
from solidation.__main__ import sanitize_md


def test_sanitize_md() -> None:
assert sanitize_md(r"[gh-actions](deps): Fix \n") \
== r"\[gh-actions\](deps): Fix \\n"
13 changes: 9 additions & 4 deletions tox.ini
Original file line number Diff line number Diff line change
@@ -1,8 +1,13 @@
[tox]
envlist = lint,typing
envlist = lint,typing,py3
isolated_build = True
minversion = 3.3.0

[testenv]
extras = test
commands =
python -m pytest -v {posargs} tests

[testenv:lint]
skip_install = True
deps =
Expand All @@ -11,13 +16,13 @@ deps =
flake8-builtins
flake8-unused-arguments
commands =
flake8 src
flake8 src tests

[testenv:typing]
deps =
mypy
commands =
mypy src
mypy src tests

[flake8]
doctests = True
Expand All @@ -37,4 +42,4 @@ lines_between_sections = 0
profile = black
reverse_relative = True
sort_relative_in_force_sorted_sections = True
src_paths = src
src_paths = src tests
yarikoptic marked this conversation as resolved.
Show resolved Hide resolved