PR#1086: hub: unittest for get_external_repos - koji

koji

#1086 hub: unittest for get_external_repos

Merged 5 years ago by mikem. Opened 5 years ago by julian8628.

julian8628/koji issue/1077 into master

hub: unittest for get_external_repos

Yu Ming Zhu • 5 years ago

a3ff86c

tests/test_hub/test_get_external_repos.py

file added

+148

		`@@ -0,0 +1,148 @@`
		`+ from __future__ import absolute_import`
		`+`
		`+ import koji`
		`+ import kojihub`
		`+ from .utils import DBQueryTestCase`
		`+`
		`+`
		`+ class TestGetExternalRepos(DBQueryTestCase):`
		`+ maxDiff = None`
		`+`
		`+ def test_get_external_repos(self):`
		`+ self.qp_execute_return_value = [{'id': 1,`
		`+ 'name': 'ext_repo_1',`
		`+ 'url': 'http://example.com/repo/'}]`
		`+ rv = kojihub.get_external_repos()`
		`+ self.assertEqual(len(self.queries), 1)`
		`+ self.assertLastQueryEqual(tables=['external_repo'],`
		`+ columns=['id', 'name', 'url'],`
		`+ joins=[`
		`+ 'external_repo_config ON external_repo_id = id'],`
		`+ clauses=['(active = TRUE)'],`
		`+ values={'clauses': ['(active = TRUE)'],`
		`+ 'event': None,`
		`+ 'fields': ['id', 'name', 'url'],`
		`+ 'info': None,`
		`+ 'joins': [`
		`+ 'external_repo_config ON external_repo_id = id'],`
		`+ 'queryOpts': None,`
		`+ 'tables': ['external_repo'],`
		`+ 'url': None},`
		`+ opts={})`
		`+ self.assertEqual(rv, [{'id': 1,`
		`+ 'name': 'ext_repo_1',`
		`+ 'url': 'http://example.com/repo/'}])`
		`+`
		`+ def test_get_external_repos_event(self):`
		`+ self.qp_execute_return_value = [{'id': 1,`
		`+ 'name': 'ext_repo_1',`
		`+ 'url': 'http://example.com/repo/'}]`
		`+ rv = kojihub.get_external_repos(event=1000)`
		`+ self.assertEqual(len(self.queries), 1)`
		`+ self.assertLastQueryEqual(tables=['external_repo'],`
		`+ columns=['id', 'name', 'url'],`
		`+ joins=[`
		`+ 'external_repo_config'`
		`+ ' ON external_repo_id = id'],`
		`+ clauses=[`
		`+ '(create_event <= 1000'`
		`+ ' AND ( revoke_event IS NULL'`
		`+ ' OR 1000 < revoke_event ))'],`
		`+ values={'clauses': [`
		`+ '(create_event <= 1000'`
		`+ ' AND ( revoke_event IS NULL'`
		`+ ' OR 1000 < revoke_event ))'],`
		`+ 'event': 1000,`
		`+ 'fields': ['id', 'name', 'url'],`
		`+ 'info': None,`
		`+ 'joins': [`
		`+ 'external_repo_config ON'`
		`+ ' external_repo_id = id'],`
		`+ 'queryOpts': None,`
		`+ 'tables': ['external_repo'],`
		`+ 'url': None},`
		`+ opts={})`
		`+ self.assertEqual(rv, [{'id': 1,`
		`+ 'name': 'ext_repo_1',`
		`+ 'url': 'http://example.com/repo/'}])`
		`+`
		`+ def test_get_external_repos_by_name(self):`
		`+ self.qp_execute_return_value = [{'id': 1,`
		`+ 'name': 'ext_repo_1',`
		`+ 'url': 'http://example.com/repo/'}]`
		`+ rv = kojihub.get_external_repos(info='ext_repo_1')`
		`+ self.assertEqual(len(self.queries), 1)`
		`+ self.assertLastQueryEqual(tables=['external_repo'],`
		`+ columns=['id', 'name', 'url'],`
		`+ joins=[`
		`+ 'external_repo_config ON external_repo_id = id'],`
		`+ clauses=['(active = TRUE)',`
		`+ 'name = %(info)s'],`
		`+ values={'clauses': ['(active = TRUE)',`
		`+ 'name = %(info)s'],`
		`+ 'event': None,`
		`+ 'fields': ['id', 'name', 'url'],`
		`+ 'info': 'ext_repo_1',`
		`+ 'joins': [`
		`+ 'external_repo_config ON external_repo_id = id'],`
		`+ 'queryOpts': None,`
		`+ 'tables': ['external_repo'],`
		`+ 'url': None},`
		`+ opts={})`
		`+ self.assertEqual(rv, [{'id': 1,`
		`+ 'name': 'ext_repo_1',`
		`+ 'url': 'http://example.com/repo/'}])`
		`+`
		`+ def test_get_external_repos_by_id(self):`
		`+ self.qp_execute_return_value = [{'id': 1,`
		`+ 'name': 'ext_repo_1',`
		`+ 'url': 'http://example.com/repo/'}]`
		`+ rv = kojihub.get_external_repos(info=1)`
		`+ self.assertEqual(len(self.queries), 1)`
		`+ self.assertLastQueryEqual(tables=['external_repo'],`
		`+ columns=['id', 'name', 'url'],`
		`+ joins=[`
		`+ 'external_repo_config ON external_repo_id = id'],`
		`+ clauses=['(active = TRUE)',`
		`+ 'id = %(info)i'],`
		`+ values={'clauses': ['(active = TRUE)',`
		`+ 'id = %(info)i'],`
		`+ 'event': None,`
		`+ 'fields': ['id', 'name', 'url'],`
		`+ 'info': 1,`
		`+ 'joins': [`
		`+ 'external_repo_config ON external_repo_id = id'],`
		`+ 'queryOpts': None,`
		`+ 'tables': ['external_repo'],`
		`+ 'url': None},`
		`+ opts={})`
		`+ self.assertEqual(rv, [{'id': 1,`
		`+ 'name': 'ext_repo_1',`
		`+ 'url': 'http://example.com/repo/'}])`
		`+`
		`+ def test_get_external_repos_by_url(self):`
		`+ self.qp_execute_return_value = [{'id': 1,`
		`+ 'name': 'ext_repo_1',`
		`+ 'url': 'http://example.com/repo/'}]`
		`+ rv = kojihub.get_external_repos(url='http://example.com/repo/')`
		`+ self.assertEqual(len(self.queries), 1)`
		`+ self.assertLastQueryEqual(tables=['external_repo'],`
		`+ columns=['id', 'name', 'url'],`
		`+ joins=[`
		`+ 'external_repo_config ON external_repo_id = id'],`
		`+ clauses=['(active = TRUE)',`
		`+ 'url = %(url)s'],`
		`+ values={'clauses': ['(active = TRUE)',`
		`+ 'url = %(url)s'],`
		`+ 'event': None,`
		`+ 'fields': ['id', 'name', 'url'],`
		`+ 'info': None,`
		`+ 'joins': [`
		`+ 'external_repo_config ON external_repo_id = id'],`
		`+ 'queryOpts': None,`
		`+ 'tables': ['external_repo'],`
		`+ 'url': 'http://example.com/repo/'},`
		`+ opts={})`
		`+ self.assertEqual(rv, [{'id': 1,`
		`+ 'name': 'ext_repo_1',`
		`+ 'url': 'http://example.com/repo/'}])`

julian8628 commented 5 years ago

relates: #1077
Since this call is a query API, and if no external_repo items found, empty list should be returned.
There's no significant reason to raise Error or add strict behavior in this case.

mikem commented 5 years ago

See my comments on #1077. I'm not sure this is an sensible call to add a strict option to.

Also, this change will also raise an error when the call is used without parameters (query all ext repos) and there are no ext repos defined in the system. This is a different case than #1077 and #1078 complain about.

julian8628 commented 5 years ago

See my comments on #1077. I'm not sure this is an sensible call to add a strict option to.
Also, this change will also raise an error when the call is used without parameters (query all ext repos) and there are no ext repos defined in the system. This is a different case than #1077 and #1078 complain about.

I agree with you that this call shouldn't have a strict behavior like that. I will change this PR having only unittests

rebased onto 511dddbd79e1f43260a2f84909cc3e13d7847771

5 years ago

rebased onto a3ff86c

5 years ago

julian8628 commented 5 years ago

updated

mikem commented 5 years ago

Commit d6b7832 fixes this pull-request