PR#684: Fix failing frontend tests, see #683 - copr/copr

copr / copr

#684 Fix failing frontend tests, see #683

Closed 5 years ago by praiskup. Opened 5 years ago by frostyx.

copr/ frostyx/copr fix-failing-tests into master

[frontend] fix a failing test

Jakub Kadlčík • 5 years ago

82180d1

[frontend] expect the correct status code for project deletion failure

Jakub Kadlčík • 5 years ago

70f05cd

frontend/coprs_frontend/tests/coprs_test_case.py

file modified

+1 -1

		`@@ -287,7 +287,7 @@`
		`source_type=helpers.BuildSourceEnum('scm'))`

		`@pytest.fixture`
		`- def f_build_few_chroots(self):`
		`+ def f_build_few_chroots(self, f_mock_chroots_many):`
		`"""`
		`Requires fixture: f_mock_chroots_many`
		`"""`

frontend/coprs_frontend/tests/test_api/test_project_r.py

file modified

+1 -1

		`@@ -330,7 +330,7 @@`
		`href,`
		`method="delete"`
		`)`
		`- assert r0.status_code == 204`
		`+ assert r0.status_code == 400`

		`def test_project_delete_fail_unfinished_project_action(`
		`self, f_users, f_mock_chroots,`

frontend/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_builds.py

file modified

+6 -6

		`@@ -124,10 +124,15 @@`
		`class TestCoprDeleteBuild(CoprsTestCase):`

		`@TransactionDecorator("u1")`
		`- def test_copr_build_submitter_can_delete_build_old(self, f_users, f_coprs, f_build_few_chroots):`
		`+ def test_copr_build_submitter_can_delete_build_old(self, f_users, f_coprs, f_build_few_chroots, f_db):`
		`self.db.session.add_all([self.u1, self.c1, self.b_few_chroots])`
		`+ self.b_few_chroots.build_chroots[1].status= StatusEnum("canceled")`
		`self.db.session.commit()`

		`+ expected_chroot_builddirs = {'srpm-builds': self.b_few_chroots.result_dir}`
		`+ for chroot in self.b_few_chroots.build_chroots:`
		`+ expected_chroot_builddirs[chroot.name] = chroot.result_dir`
		`+`
		`expected_dir = self.b_few_chroots.result_dir`
		`r = self.test_client.post(`
		`"/coprs/{0}/{1}/delete_build/{2}/"`
		`@@ -144,11 +149,6 @@`
		`assert act.object_type == "build"`
		`assert data.get('ownername') == "user1"`
		`assert data.get('projectname') == "foocopr"`
		`-`
		`- expected_chroot_builddirs = {'srpm-builds': self.b_few_chroots.result_dir}`
		`- for chroot in self.b_few_chroots.build_chroots:`
		`- expected_chroot_builddirs[chroot.name] = chroot.result_dir`
		`-`
		`assert json.loads(act.data)["chroot_builddirs"] == expected_chroot_builddirs`

		`@TransactionDecorator("u1")`

frostyx commented 5 years ago

See #683

praiskup commented on line 6 of frontend/coprs_frontend/tests/test_views/test_coprs_ns/test_coprs_builds.py 5 years ago

f_build_few_chroots documents it requires f_mock_chroots_many, what about to rather directly depend on it (put f_mock_chroots_many into f_build_few_chroots arguments)?

praiskup commented 5 years ago

Commit 1. is IMO wrong; it simulates the situation when build has been submitted, it is not yet finished - but user in the meantime disabled the mock-chroot in the project. So there are builds assigned to some build_chroots, while the copr project is not assigned to corresponding MockChroot.

Commit 2 seems to be correct; the commit c2fcfff only hacked testsuite to not fail, but it caused a hidden bug.

Commit 3 seems to be wrong; and test_copr_build_submitter_can_delete_build_old seems to expect that in self.b_few_chroots.id build are no unfinished builds, which is not true in f_build_few_chroots fixture ... (i.e. as you correctly noted, for the purpose of the test we should move all the biulds to some finished state).

praiskup commented 5 years ago

Commit 3 seems to be wrong; and test_copr_build_submitter_can_delete_build_old seems to expect that in self.b_few_chroots.id build are no unfinished builds, which is not true in f_build_few_chroots fixture ... (i.e. as you correctly noted, for the purpose of the test we should move all the biulds to some finished state).

Ok, taking back this one. It is probably correct change (adding at least one
build_chroot to the build to be deleted).

Edited 5 years ago by praiskup

rebased onto 7419f23db6c549691ecb2ffbc8c6b63484fbba90

5 years ago

frostyx commented 5 years ago

I've updated the tests. Please take a look.

rebased onto 70f05cd

5 years ago

frostyx commented 5 years ago

I've removed the first commit