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

Rewrite test_sync.py using pytest #334

Merged
merged 1 commit into from
Jan 31, 2024
Merged

Conversation

git-hyagi
Copy link
Contributor

fixes: #327

@git-hyagi git-hyagi marked this pull request as draft January 26, 2024 19:05
@git-hyagi git-hyagi marked this pull request as ready for review January 30, 2024 14:10
Copy link
Member

@lubosmj lubosmj left a comment

Choose a reason for hiding this comment

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

Good job! Please, revisit my comments.

repo = ostree_repositories_api_client.read(repo.pulp_href)
return ostree_repositories_versions_api_client.read(repo_version_href), remote, repo

def _synced_repo_version(repo=None, remote=None, policy="immediate"):
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
def _synced_repo_version(repo=None, remote=None, policy="immediate"):
def _sync_repo_version(repo=None, remote=None, policy="immediate"):

And "sync_repo_version".

Comment on lines 99 to 101
assert KeyError, lambda: added_content["ostree.refs"]
assert KeyError, lambda: added_content["ostree.commit"]
assert KeyError, lambda: added_content["ostree.object"]
Copy link
Member

Choose a reason for hiding this comment

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

These statements are always evaluated to True.

Try using pytest.raises instead, please: https://docs.pytest.org/en/7.1.x/reference/reference.html#pytest.raises.

Comment on lines 150 to 151
repo_version_href = repo.latest_version_href
repo = ostree_repositories_api_client.read(repo.pulp_href)
Copy link
Member

Choose a reason for hiding this comment

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

A safe bet would be running repo = ostree_repositories_api_client.read(repo.pulp_href) right after monitor_task in case the task's logic fails (defensive programming). With that, repo_version_href = repo.latest_version_href will definitely return the latest version of the repository.

@git-hyagi git-hyagi force-pushed the pytest-refactor-sync branch 2 times, most recently from 2c0275b to 648744c Compare January 31, 2024 18:04
@lubosmj lubosmj merged commit 720412c into pulp:main Jan 31, 2024
16 checks passed
@git-hyagi git-hyagi deleted the pytest-refactor-sync branch January 31, 2024 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rewrite test_sync.py to pytest
2 participants