t: Add tests for AggregateResultsSync #242
Conversation
okurz
force-pushed
the
feature/increase_test_coverage3
branch
from
25887fb to
7440e77
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #242 +/- ##
==========================================
+ Coverage 73.92% 75.09% +1.17%
==========================================
Files 30 30
Lines 2646 2646
==========================================
+ Hits 1956 1987 +31
+ Misses 690 659 -31 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
okurz
changed the title
okurz
changed the title
okurz
force-pushed
the
feature/increase_test_coverage3
branch
from
5b81d16 to
742a933
Compare
| sync.client.get_jobs.return_value = [{"id": 1}] | ||
| assert sync() == 0 | ||
| mock_normalize.assert_not_called() | ||
| mock_post.assert_not_called() |
There was a problem hiding this comment.
This test should check what jobs have been posted. Otherwise the added coverage is misleading us into thinking this part of the code has tests that would fail if we break something.
There was a problem hiding this comment.
Could you provide more specific suggestions because I don't understand the code enough yet to be able to solve that easily
There was a problem hiding this comment.
You can check the tests for Gitea. There I didn't completely mock away the client but used responses and its API to check what calls were made. (But maybe you can also change @patch("openqabot.aggrsync.SyncRes.post_result") so that invocations are recorded somehow. I didn't use @patch so far so I don't know what's possible with that.)
| args = Namespace( | ||
| configs=Path("bar"), | ||
| dry=True, | ||
| token="null", | ||
| instance="null", | ||
| openqa_instance=urlparse("http://localhost"), | ||
| ) | ||
| sync = AggregateResultsSync(args) |
There was a problem hiding this comment.
Maybe these repeated lines could go into a helper function.
There was a problem hiding this comment.
Yes, I am wondering about what is the best approach for reusing. Should we use a custom helper function or a test fixture following https://docs.pytest.org/en/7.1.x/how-to/fixtures.html#fixtures-can-request-other-fixtures
There was a problem hiding this comment.
I don't care. A custom helper function would probably be the easiest/simplest solution (and therefore also the most readable).
okurz
force-pushed
the
feature/increase_test_coverage3
branch
from
742a933 to
4d233cd
Compare
okurz
changed the title
okurz
force-pushed
the
feature/increase_test_coverage3
branch
from
4d233cd to
c760cc9
Compare
Add initial tests for the AggregateResultsSync class, covering the constructor and a basic call with no settings.
okurz
force-pushed
the
feature/increase_test_coverage3
branch
from
c760cc9 to
05a74b4
Compare
2 participants
No description provided.