test_landings: add test_deferred_jobs (bug 2021883)#986
test_landings: add test_deferred_jobs (bug 2021883)#986zzzeid wants to merge 1 commit intozeid/bug-2022618-repo-mc-fixfrom
Conversation
|
View this pull request in Lando to land it once approved. |
zzzeid
force-pushed
the
zeid/bug-2021883-deferred-jobs-tests
branch
3 times, most recently
from
751aad8 to
53524a6
Compare
zzzeid
force-pushed
the
zeid/bug-2022618-repo-mc-fix
branch
from
f5ea52b to
457e3c1
Compare
- add a patch that can be applied over and over again - add test that simulates a queue with two different repos
zzzeid
force-pushed
the
zeid/bug-2021883-deferred-jobs-tests
branch
from
53524a6 to
3fb19ef
Compare
| # This test simulates a number of jobs in two different repos | ||
| # being submitted, and ensures that when closing a tree the | ||
| # queue is not blocked by the jobs in the queue that target | ||
| # the closed tree. |
There was a problem hiding this comment.
Nit: this could be a docstring comment.
| if number == 3: | ||
| return PATCH_NORMAL_4.format(filename=uuid.uuid4()) |
There was a problem hiding this comment.
Since this new infinitely-applying patch is a special case, I think it would be better to create a new success_patch or something similar, instead of shoehorning it into normal_patch only when the input argument is 3.
If there's some reason this should be kept in normal_patch, we should write a comment explaining why, and find a better way to pick this patch instead of making 3 a magic-number.
|
|
||
|
|
||
| @pytest.mark.django_db | ||
| def test_deferred_jobs( |
There was a problem hiding this comment.
test_deferred_jobs is not very descriptive. How about test_closed_tree_defers_jobs_without_blocking_other_repos?
| job.status == JobStatus.LANDED for job in jobs[3:] | ||
| ), "Remaining repo2 jobs should be landed." | ||
|
|
||
| refresh(jobs) |
There was a problem hiding this comment.
No need to refresh here, as we haven't changed anything since the previous refresh.
| git_worker.start(max_loops=10) | ||
| refresh(jobs) | ||
| assert all( | ||
| job.status == JobStatus.LANDED for job in jobs[3:] |
There was a problem hiding this comment.
Checking the job statuses by slicing the jobs list is brittle. If someone needs to extend the test to include another job they'll have to tweak these slicing values based on the insertion order into the main jobs list.
We should instead create a separate list of repo1/repo2 jobs and check the behaviour in those lists instead:
repo1_jobs = [job for job in jobs if job.target_repo == git_repo_1]
repo2_jobs = [job for job in jobs if job.target_repo == git_repo_2]
assert all(
job.status == JobStatus.LANDED for job in repo2_jobs
), "All repo2 jobs should be landed."
assert repo1_jobs[0].status == JobStatus.DEFERRED, "First remaining repo1 job should be deferred."
| if number == 3: | ||
| return PATCH_NORMAL_4.format(filename=uuid.uuid4()) |
| job_params["target_repo"] = git_repo_1 | ||
| jobs = [] | ||
| for i in (1, 2, 3): | ||
| jobs.append( | ||
| make_landing_job( | ||
| revisions=[ | ||
| create_patch_revision(1, patch=normal_patch(3), revision_id=i) | ||
| ], | ||
| **job_params, |
There was a problem hiding this comment.
We might as well pass target_repo directly, without using job_params?
| job_params["target_repo"] = git_repo_1 | |
| jobs = [] | |
| for i in (1, 2, 3): | |
| jobs.append( | |
| make_landing_job( | |
| revisions=[ | |
| create_patch_revision(1, patch=normal_patch(3), revision_id=i) | |
| ], | |
| **job_params, | |
| jobs = [] | |
| for i in (1, 2, 3): | |
| jobs.append( | |
| make_landing_job( | |
| revisions=[ | |
| create_patch_revision(1, patch=normal_patch(3), revision_id=i) | |
| ], | |
| target_repo=git_repo_1, |
| job_params["target_repo"] = git_repo_2 | ||
| for i in (4, 5, 6): | ||
| jobs.append( | ||
| make_landing_job( | ||
| revisions=[ | ||
| create_patch_revision(1, patch=normal_patch(3), revision_id=i) | ||
| ], | ||
| **job_params, |
There was a problem hiding this comment.
Ditto
| job_params["target_repo"] = git_repo_2 | |
| for i in (4, 5, 6): | |
| jobs.append( | |
| make_landing_job( | |
| revisions=[ | |
| create_patch_revision(1, patch=normal_patch(3), revision_id=i) | |
| ], | |
| **job_params, | |
| for i in (4, 5, 6): | |
| jobs.append( | |
| make_landing_job( | |
| revisions=[ | |
| create_patch_revision(1, patch=normal_patch(3), revision_id=i) | |
| ], | |
| target_repo: git_repo_2, |
| git_worker.start(max_loops=10) | ||
| refresh(jobs) | ||
| assert all( | ||
| job.status == JobStatus.LANDED for job in jobs[3:] |
3 participants
Uh oh!
There was an error while loading. Please reload this page.