Skip to content

Commit

Permalink
Furthering of typing fixes.
Browse files Browse the repository at this point in the history
  • Loading branch information
DFINITYManu committed Jan 16, 2025
1 parent 220f427 commit cc4bee7
Show file tree
Hide file tree
Showing 16 changed files with 64 additions and 43 deletions.
4 changes: 4 additions & 0 deletions docs/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,10 @@ rye run <command>

to run the `<command>` with all expected dependencies.

### IDE setup

Point your IDE to the Python interpreter inside `.venv/bin`.

### Troubleshooting rye

If you face problems in `rye sync`, such as `unknown version cpython@...`, you can try to
Expand Down
3 changes: 2 additions & 1 deletion mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ strict = True
namespace_packages = True
ignore_missing_imports = True
follow_imports = silent
follow_imports_for_stubs = True
follow_imports_for_stubs = True
mypy_path = $MYPY_CONFIG_FILE_DIR/release-controller
3 changes: 2 additions & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ dependencies = [
"google-auth-oauthlib>=1.2.1",
"pydrive2>=1.20.0",
"markdownify>=0.13.1",
"pytest>=8.3.2",
"pytest>=8.3.4",
"pytest-asyncio>=0.24.0",
"pytest-xdist>=3.6.1",
"git-changelog>=2.5.2",
Expand All @@ -49,6 +49,7 @@ dependencies = [
"ruamel-yaml>=0.18.6",
"httpretty>=1.1.4",
"aiohttp>=3.10.5",
"uv>=0.5.20",
]

[tool.rye]
Expand Down
26 changes: 9 additions & 17 deletions release-controller/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -36,10 +36,7 @@ py_binary(
main = "reconciler.py",
srcs = glob(
["*.py"],
exclude = [
"test_*.py",
"pytest.py",
],
exclude=["pytest.py"]
),
tags = ["typecheck"],
env = env,
Expand All @@ -51,10 +48,6 @@ py_binary(
main = "commit_annotator.py",
srcs = glob(
["*.py"],
exclude = [
"test_*.py",
"pytest.py",
],
),
data = [":bazelisk", ":target_determinator"],
env = env,
Expand All @@ -78,29 +71,28 @@ native_binary(
)

long_tests = [
"test_commit_annotator.py",
"tests/test_commit_annotator.py",
]

py_test(
name = "pytest",
srcs = ["pytest.py"],
data = glob(["*.py"], exclude = long_tests) + glob(["test_data/*"]),
srcs = ["pytest.py"] + glob(["tests/*.py"], exclude = long_tests + ["tests/test_reconciler.py"]) + glob(["*.py"], exclude = ["pytest.py"]),
data = glob(["tests/test_data/*"]),
env = env,
tags = ["no-sandbox", "typecheck"],
tags = ["no-sandbox"],
deps = deps + dev_deps,
env_inherit = ["HOME"],
)

py_test(
name = "pytest_enormous",
srcs = ["pytest.py"],
name = "test_reconciler",
srcs = ["pytest.py"] + ["tests/test_reconciler.py"] + glob(["*.py"], exclude = ["pytest.py"]),
data = glob(["tests/test_data/*"]),
main = "pytest.py",
data = long_tests + glob(["*.py"], exclude = ["test*.py"]) + [":bazelisk", ":target_determinator"],
env = env,
tags = ["no-sandbox", "manual"],
tags = ["no-sandbox", "typecheck"],
deps = deps + dev_deps,
env_inherit = ["HOME"],
size = "enormous",
)

tar(
Expand Down
16 changes: 14 additions & 2 deletions release-controller/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,6 +159,9 @@ Make sure also that few minutes have passed and that public dashboard still does

## Development

Please see the parent folder's `README.md` for virtual environment setup.
Follow the whole *Contributing* section to the letter.

### Running the reconciler in dry-run mode:

```sh
Expand Down Expand Up @@ -204,13 +207,22 @@ testing your changes? Add `--output_groups=-mypy` right after `bazel run`.

### Tests

Unit tests:
#### Unit tests

```sh
bazel test //release-controller:pytest
```

Mypy typing tests:
With the .venv setup, you can also run (with varying levels of success):

```sh
.venv/bin/python3 -m pytest release-controller/
```

The above runs all tests. If you want to run a specific test file,
specify it as a path instead of the folder specified above.

#### Typing correctness

```sh
bazel build //release-controller:release-controller
Expand Down
2 changes: 1 addition & 1 deletion release-controller/reconciler_state.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ class ReconcilerState:
def __init__(
self,
path: pathlib.Path,
known_proposals: list[dre_cli.ElectionProposal] | None,
known_proposals: list[dre_cli.ElectionProposal] | None = None,
):
"""
Create a new state object.
Expand Down
File renamed without changes.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ def test_publish_if_ready__ready(mocker):

publish_client.publish_if_ready(
google_doc_to_markdown(
pathlib.Path(os.path.dirname(__file__)) / "test_data" / "b0ade55f7e8999e2842fe3f49df163ba224b71a2.docx"
pathlib.Path(os.path.dirname(__file__))
/ "test_data"
/ "b0ade55f7e8999e2842fe3f49df163ba224b71a2.docx"
),
"b0ade55f7e8999e2842fe3f49df163ba224b71a2",
)
Expand Down Expand Up @@ -347,6 +349,7 @@ def test_publish_if_ready__not_ready2(mocker):

assert publish_client.ensure_published.call_count == 0 # pylint: disable=no-member


def test_publish_if_ready__ready_no_changes(mocker):
github_client = Github()
mocker.patch.object(github_client, "get_repo")
Expand Down Expand Up @@ -390,7 +393,7 @@ def test_publish_if_ready__ready_no_changes(mocker):
* ~~author: Leo Eich |~~ [e76c5a374](https://github.com/dfinity/ic/commit/e76c5a374) Consensus(ecdsa): Stop relaying tECDSA signature shares
* ~~author: Leo Eich |~~ [2d63da24c](https://github.com/dfinity/ic/commit/2d63da24c) Consensus(ecdsa): Add optional kappa\\_unmasked config to QuadrupleInCreation
""",
"2e921c9adfc71f3edc96a9eb5d85fc742e7d8a9f"
"2e921c9adfc71f3edc96a9eb5d85fc742e7d8a9f",
)

assert publish_client.ensure_published.call_count == 0 # pylint: disable=no-member
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
import pathlib
import tempfile
from types import SimpleNamespace
from unittest.mock import Mock
import pytest_mock.plugin

import os
import sys

sys.path.append(os.path.join(os.path.dirname(__file__), os.path.pardir))

import git_repo
import pytest
import release_index
import requests
import typing
from forum import ReleaseCandidateForumClient
from github import Github
Expand All @@ -22,16 +26,14 @@
from release_index_loader import StaticReleaseLoader


class TestReconcilerState(ReconcilerState):
class AmnesiacReconcilerState(ReconcilerState):
"""Reconciler state that uses a temporary directory for storage."""

def __init__(self):
"""Create a new TestReconcilerState."""
def __init__(self) -> None:
self.tempdir = tempfile.TemporaryDirectory()
super().__init__(pathlib.Path(self.tempdir.name))

def __del__(self):
"""Clean up the temporary directory."""
def __del__(self) -> None:
self.tempdir.cleanup()


Expand All @@ -53,8 +55,8 @@ def replica_version_proposals(self) -> dict[str, int]:
return self.rep


@pytest.mark.skip(reason="not finished")
def test_e2e_mock_new_release(mocker):
@pytest.mark.skip(reason="not finished") # type: ignore
def test_e2e_mock_new_release(mocker: pytest_mock.plugin.MockerFixture) -> None:
"""Test the workflow when a new release is added to the index."""
discourse_client = DiscourseClientMock()
forum_client = ReleaseCandidateForumClient(discourse_client)
Expand Down Expand Up @@ -84,7 +86,7 @@ def test_e2e_mock_new_release(mocker):
loader=StaticReleaseLoader(config),
publish_client=PublishNotesClient(repo),
nns_url="",
state=TestReconcilerState(),
state=AmnesiacReconcilerState(),
ic_repo=ic_repo_mock,
ignore_releases=[""],
# FIXME: mock next two lines properly
Expand Down Expand Up @@ -165,7 +167,7 @@ def test_e2e_mock_new_release(mocker):
assert len(discourse_client.created_topics) == 1


def test_versions_to_unelect():
def test_versions_to_unelect() -> None:
index = parse_yaml_raw_as(
release_index.Model,
"""
Expand Down Expand Up @@ -248,7 +250,7 @@ def test_versions_to_unelect():
]


def test_oldest_active_release():
def test_oldest_active_release() -> None:
index = parse_yaml_raw_as(
release_index.Model,
"""
Expand Down Expand Up @@ -320,7 +322,7 @@ def iter_content(self, chunk_size: int = 1024) -> typing.Iterator[bytes]:
return mock_request


def test_version_package_checksum(mocker: typing.Any) -> None:
def test_version_package_checksum(mocker: pytest_mock.plugin.MockerFixture) -> None:
def content_getter(url: str) -> str:
if url.endswith("SHA256SUMS"):
return """\
Expand All @@ -341,7 +343,9 @@ def content_getter(url: str) -> str:
)


def test_version_package_checksum_mismatch(mocker: typing.Any) -> None:
def test_version_package_checksum_mismatch(
mocker: pytest_mock.plugin.MockerFixture,
) -> None:
def content_getter(url: str) -> str:
if url.endswith("SHA256SUMS"):
return """\
Expand All @@ -357,12 +361,12 @@ def content_getter(url: str) -> str:

mocker.patch("requests.get", new=mock_request_get(content_getter))

with pytest.raises(Exception) as e:
with pytest.raises(Exception) as e: # type: ignore
version_package_checksum("notimporant")
assert "do not match contents" in str(e.value)


def test_find_base_release():
def test_find_base_release() -> None:
with tempfile.TemporaryDirectory() as repo_cache_dir:
ic_repo = git_repo.GitRepo(
"https://github.com/dfinity/ic.git",
Expand Down
File renamed without changes.
4 changes: 3 additions & 1 deletion release-controller/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,9 @@ def __len__(self) -> int:
return int(math.ceil((self.len / self.chunk_size)))

def __iter__(self) -> typing.Iterator[bytes]:
return self.resp.iter_content(chunk_size=self.chunk_size)
return typing.cast(
typing.Iterator[bytes], self.resp.iter_content(chunk_size=self.chunk_size)
)

def __contains__(self, item: typing.Any) -> bool:
return False
Expand Down
3 changes: 2 additions & 1 deletion requirements-dev.lock
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ pyparsing==3.1.4
# via matplotlib
pyreadline3==3.4.1 ; sys_platform == 'win32'
# via humanfriendly
pytest==8.3.2
pytest==8.3.4
# via pytest-asyncio
# via pytest-mock
# via pytest-xdist
Expand Down Expand Up @@ -608,6 +608,7 @@ urllib3==2.2.2
# via elastic-transport
# via pygithub
# via requests
uv==0.5.20
virtualenv==20.26.3
# via pre-commit
waiter==1.5
Expand Down
3 changes: 2 additions & 1 deletion requirements.lock
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ pyparsing==3.1.4
# via matplotlib
pyreadline3==3.4.1 ; sys_platform == 'win32'
# via humanfriendly
pytest==8.3.2
pytest==8.3.4
# via pytest-asyncio
# via pytest-mock
# via pytest-xdist
Expand Down Expand Up @@ -608,6 +608,7 @@ urllib3==2.2.2
# via elastic-transport
# via pygithub
# via requests
uv==0.5.20
virtualenv==20.26.3
# via pre-commit
waiter==1.5
Expand Down

0 comments on commit cc4bee7

Please sign in to comment.