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
6 changes: 6 additions & 0 deletions setup.cfg
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ install_requires =
PyGithub ~= 2.0
ruamel.yaml ~= 0.15

[options.extras_require]
test =
mypy
yarikoptic marked this conversation as resolved.
Show resolved Hide resolved
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
7 changes: 6 additions & 1 deletion 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 Down Expand Up @@ -437,6 +437,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'[\[\]]', '', s)
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't it be better to instead escape brackets, changing [ to \[?

Copy link
Member Author

Choose a reason for hiding this comment

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

dang, I didn't even try but it indeed seems to work:

so I guess we would also need to escape the \:

ok, will do... now



@click.command()
@click.option(
"-c",
Expand Down
Empty file.
5 changes: 5 additions & 0 deletions src/solidation/tests/test_main.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
from ..__main__ import sanitize_md
Copy link
Member

Choose a reason for hiding this comment

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

Move this file to tests/test_main.py relative to the project root (i.e., outside of src/).

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 really see little benefits and only inconveniences from such separation, but ok, will do, since you are the lead on this one.

Copy link
Member Author

Choose a reason for hiding this comment

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

done in 4978b8a which also required replicating listing of tests now for all other tox components which take care about source code



def test_sanitize_md() -> None:
assert sanitize_md("[gh-actions](deps): Bump") == "gh-actions(deps): Bump"
7 changes: 6 additions & 1 deletion 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} src
Copy link
Member

Choose a reason for hiding this comment

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

Change src to tests after moving the test file.


[testenv:lint]
skip_install = True
deps =
Expand Down