Skip to content

Conversation

@theihor
Copy link
Contributor

@theihor theihor commented Jun 11, 2025

Before implementing #6 I wanted to have a good test coverage of the relevant code.
To get that, I wanted to add a number of tests, and to enable that some refactoring was appropriate.

See individual commits for details.

@theihor theihor force-pushed the sync-test branch 2 times, most recently from 07e3f3f to 4a144f7 Compare June 12, 2025 20:00
@theihor theihor changed the title tests: add test_sync_patches_pr_summary_success Add more github_sync unit tests Jun 13, 2025
Copy link

@danielocfb danielocfb left a comment

Choose a reason for hiding this comment

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

Looks good to me overall.

theihor added 4 commits June 16, 2025 15:00
Add a comprehensive test covering most of the sync_patches logic.

Extend patchwork_mock to be able to load test responses from disk if
appropriate. Use realistic json responses for this test to make KPD
trigger relevant http calls.

Signed-off-by: Ihor Solodrai <[email protected]>
Factor out sync_relevant_subject() routine from sync_patches()

Add test cases verifying base branch selection logic, when applying
the series.

Signed-off-by: Ihor Solodrai <[email protected]>
A couple of helper functions refered to "series/123" part of a branch
name ("series/123=>target") as "base" which is quite confusing.

Refactor helpers that were parsing PR refs and their usage.
Move separator constants used for branch names to config.py

Signed-off-by: Ihor Solodrai <[email protected]>
Using @Property and async on the same function is confusing and makes
mocking for unit tests difficult and sometimes impossible.

Remove @Property decorator from Subject getters and make them simple
async functions. Fix usages accordingly.

Signed-off-by: Ihor Solodrai <[email protected]>
Copy link

@danielocfb danielocfb left a comment

Choose a reason for hiding this comment

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

Still looks good.

@theihor theihor merged commit 928c846 into main Jun 17, 2025
5 checks passed
@danielocfb danielocfb deleted the sync-test branch July 1, 2025 17:41
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.

3 participants