Skip to content

Commit eb7c85a

Browse files
committed
Config: deprecate gerrit_* with gerrit dict
Replace all gerrit_* parameters by the same parameters in gerrit dict (eg. gerrit_realm is becoming gerrit > realm). All gerrit_* parameters are marked as deprecated, ie. they are still supported but Rift emits warning visible by end users to report the new parameter name. Also add unit tests to cover Gerrit module.
1 parent 3aae0dd commit eb7c85a

File tree

4 files changed

+114
-12
lines changed

4 files changed

+114
-12
lines changed

lib/rift/Config.py

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -264,11 +264,31 @@ class Config():
264264
'vm_build_post_script': {
265265
'deprecated': 'vm.build_post_script'
266266
},
267-
'gerrit_realm': {},
268-
'gerrit_server': {},
269-
'gerrit_url': {},
270-
'gerrit_username': {},
271-
'gerrit_password': {},
267+
'gerrit': {
268+
'check': 'dict',
269+
'syntax': {
270+
'realm': {},
271+
'server': {},
272+
'url': {},
273+
'username': {},
274+
'password': {},
275+
}
276+
},
277+
'gerrit_realm': {
278+
'deprecated': 'gerrit.realm'
279+
},
280+
'gerrit_server': {
281+
'deprecated': 'gerrit.server'
282+
},
283+
'gerrit_url': {
284+
'deprecated': 'gerrit.url'
285+
},
286+
'gerrit_username': {
287+
'deprecated': 'gerrit.username'
288+
},
289+
'gerrit_password': {
290+
'deprecated': 'gerrit.password'
291+
},
272292
'rpm_macros': {
273293
'check': 'dict',
274294
},

lib/rift/Gerrit.py

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -80,15 +80,20 @@ def invalidate(self):
8080
def push(self, config, changeid, revid):
8181
"""Send REST request to Gerrit server from config"""
8282
auth_methods = ('digest', 'basic')
83-
realm = config.get('gerrit_realm')
84-
server = config.get('gerrit_server')
85-
username = config.get('gerrit_username')
86-
password = config.get('gerrit_password')
87-
auth_method = config.get('gerrit_auth_method', 'basic')
83+
84+
gerrit_config = config.get('gerrit')
85+
if gerrit_config is None:
86+
raise RiftError("Gerrit configuration is not defined")
87+
88+
realm = gerrit_config.get('realm')
89+
server = gerrit_config.get('server')
90+
username = gerrit_config.get('username')
91+
password = gerrit_config.get('password')
92+
auth_method = gerrit_config.get('auth_method', 'basic')
8893

8994
if realm is None:
9095
raise RiftError("Gerrit realm is not defined")
91-
if server is None and config.get('gerrit_url') is None:
96+
if server is None and gerrit_config.get('url') is None:
9297
raise RiftError("Gerrit url is not defined")
9398
if username is None:
9499
raise RiftError("Gerrit username is not defined")
@@ -98,7 +103,7 @@ def push(self, config, changeid, revid):
98103
raise RiftError(f"Gerrit auth_method is not correct (supported {auth_methods})")
99104

100105
# Set a default url if only gerrit_server was defined
101-
url = config.get('gerrit_url', f"https://{server}")
106+
url = gerrit_config.get('url', f"https://{server}")
102107

103108
# FIXME: Don't check certificate
104109
ctx = ssl.create_default_context()

tests/Config.py

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,27 @@ def test_load_deprecated_vm_parameters(self):
677677
self.assertEqual(config.get('vm').get('cpus'), 42)
678678
self.assertEqual(config.get('vm').get('memory'), 1234)
679679

680+
def test_load_deprecated_gerrit_parameters(self):
681+
"""load() deprecated gerrit_* parameters."""
682+
cfgfile = make_temp_file(
683+
textwrap.dedent(
684+
"""
685+
annex: /a/dir
686+
vm:
687+
image: /a/image.img
688+
gerrit_realm: Rift
689+
gerrit_url: https://localhost
690+
gerrit_username: rift
691+
"""
692+
)
693+
)
694+
config = Config()
695+
with self.assertWarns(RiftDeprecatedConfWarning):
696+
config.load(cfgfile.name)
697+
self.assertEqual(config.get('gerrit').get('realm'), 'Rift')
698+
self.assertEqual(config.get('gerrit').get('url'), 'https://localhost')
699+
self.assertEqual(config.get('gerrit').get('username'), 'rift')
700+
680701

681702
class ConfigTestSyntax(RiftTestCase):
682703
"""Test Config with modified syntax."""

tests/Gerrit.py

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
#
2+
# Copyright (C) 2024 CEA
3+
#
4+
from unittest import mock
5+
6+
from TestUtils import RiftTestCase
7+
from rift.Config import Config
8+
from rift.Gerrit import Review
9+
from rift import RiftError
10+
11+
class GerritTest(RiftTestCase):
12+
def setUp(self):
13+
self.config = Config()
14+
self.config.set(
15+
'gerrit',
16+
{
17+
'realm': 'Rift',
18+
'server': 'localhost',
19+
'username': 'rift',
20+
'password': 'SECR3T',
21+
}
22+
)
23+
self.review = Review()
24+
self.review.add_comment('/path/to/file', 42, 'E', 'test error message')
25+
26+
def test_invalidate(self):
27+
""" Test Review.invalidate() method"""
28+
self.assertEqual(self.review.validated, True)
29+
self.review.invalidate()
30+
self.assertEqual(self.review.validated, False)
31+
32+
@mock.patch("rift.Gerrit.urllib.urlopen")
33+
def test_push(self, mock_urlopen):
34+
""" Test Review push """
35+
self.review.push(self.config, 4242, 42)
36+
# Check push successfully send HTTP request with urllib.urlopen() and
37+
# reads its result.
38+
mock_urlopen.assert_called_once()
39+
mock_urlopen.return_value.read.assert_called_once()
40+
41+
def test_push_no_config(self):
42+
""" Test Review push w/o gerrit config error """
43+
del self.config.options['gerrit']
44+
with self.assertRaisesRegex(RiftError, "Gerrit configuration is not defined"):
45+
self.review.push(self.config, 4242, 42)
46+
47+
def test_push_missing_conf_param(self):
48+
""" Test Review push with missing parameter error """
49+
gerrit_conf = self.config.get('gerrit')
50+
for parameter, value in gerrit_conf.copy().items():
51+
# temporary remove parameter
52+
del gerrit_conf[parameter]
53+
with self.assertRaisesRegex(RiftError, "Gerrit .* is not defined"):
54+
self.review.push(self.config, 4242, 42)
55+
# restore value
56+
gerrit_conf[parameter] = value

0 commit comments

Comments
 (0)