Merge lp:~gesha/linaro-license-protection/1146220 into lp:~linaro-automation/linaro-license-protection/trunk

Proposed by Georgy Redkozubov
Status: Merged
Approved by: Stevan Radaković
Approved revision: 186
Merged at revision: 184
Proposed branch: lp:~gesha/linaro-license-protection/1146220
Merge into: lp:~linaro-automation/linaro-license-protection/trunk
Diff against target: 131 lines (+85/-0)
2 files modified
scripts/publish_to_snapshots.py (+31/-0)
tests/test_publish_to_snapshots.py (+54/-0)
To merge this branch: bzr merge lp:~gesha/linaro-license-protection/1146220
Reviewer Review Type Date Requested Status
Stevan Radaković Approve
Review via email: mp+160352@code.launchpad.net

Description of the change

This branch reinstates BUILD-INFO.txt support for publishing service.
Publishing script checks if BUILD-INFO.txt is present among artifacts being published or among already published artifacts for the same build. Otherwise publishing is aborted with an error.

To post a comment you must log in.
Revision history for this message
Stevan Radaković (stevanr) wrote :

Nice work gesha, thanks!

Only thing I dislike here is having return values(PASS, FAIL) in the method instead of using the exceptions as this is a very good example on when they should be used. That way you don't need to return anything at all, just catch exception if the method throws it. Tests would need to go through a little change as well because of it.

review: Needs Fixing (code)
184. By Georgy Redkozubov

Rewritten code to use exceptions.

Revision history for this message
Stevan Radaković (stevanr) wrote :

46 @@ -387,6 +408,7 @@
47 if build_dir_path is None or target_dir_path is None:
48 print "Problem with build/target path, move failed"
49 return FAIL
50 + publisher.check_buildinfo(build_dir_path, target_dir_path)

Think we should have here something like:

try:
    publisher.check_buildinfo(build_dir_path, target_dir_path)
except as e:
    print e.strerror
    return FAIL

185. By Georgy Redkozubov

Rewrite exception handling.

186. By Georgy Redkozubov

Fix pep8

Revision history for this message
Stevan Radaković (stevanr) wrote :

Looks good.
Approve +1

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'scripts/publish_to_snapshots.py'
2--- scripts/publish_to_snapshots.py 2013-02-26 08:38:32 +0000
3+++ scripts/publish_to_snapshots.py 2013-04-25 19:33:24 +0000
4@@ -15,6 +15,7 @@
5 product_dir_path = 'target/product'
6 PASS = 0
7 FAIL = 1
8+buildinfo = 'BUILD-INFO.txt'
9 acceptable_job_types = [
10 'android',
11 'prebuilt',
12@@ -61,6 +62,16 @@
13 pass
14
15
16+class BuildInfoException(Exception):
17+ """BUILD-INFO.txt is absent."""
18+
19+ def __init__(self, value):
20+ self.value = value
21+
22+ def __str__(self):
23+ return repr(self.value)
24+
25+
26 class SnapshotsPublisher(object):
27
28 # Files that need no sanitization even when publishing to staging.
29@@ -367,6 +378,21 @@
30 print "Failed to move files destination path", target_dir_path
31 return FAIL
32
33+ def check_buildinfo(self, build_dir_path, target_dir_path):
34+ bi_dirs = []
35+ for path, subdirs, files in os.walk(build_dir_path):
36+ for filename in files:
37+ if buildinfo in filename:
38+ bi_dirs.append(path)
39+ for path, subdirs, files in os.walk(target_dir_path):
40+ for filename in files:
41+ if buildinfo in filename:
42+ bi_dirs.append(path)
43+
44+ if not bi_dirs:
45+ raise BuildInfoException(
46+ "BUILD-INFO.txt is not present for build being published.")
47+
48
49 def main():
50 global uploads_path
51@@ -387,6 +413,11 @@
52 if build_dir_path is None or target_dir_path is None:
53 print "Problem with build/target path, move failed"
54 return FAIL
55+ try:
56+ publisher.check_buildinfo(build_dir_path, target_dir_path)
57+ except BuildInfoException as e:
58+ print e.value
59+ return FAIL
60 ret = publisher.move_artifacts(args, build_dir_path, target_dir_path)
61 if ret != PASS:
62 print "Move Failed"
63
64=== modified file 'tests/test_publish_to_snapshots.py'
65--- tests/test_publish_to_snapshots.py 2013-02-26 08:38:32 +0000
66+++ tests/test_publish_to_snapshots.py 2013-04-25 19:33:24 +0000
67@@ -11,6 +11,8 @@
68 SnapshotsPublisher,
69 setup_parser,
70 product_dir_path,
71+ BuildInfoException,
72+ buildinfo
73 )
74
75
76@@ -726,3 +728,55 @@
77 self.assertEqual(content, open(resulting_file).read())
78 self.assertEqual(howto_content, open(resulting_howto_file).read())
79 shutil.rmtree(source_dir)
80+
81+ def test_check_buildinfo_upload_dir(self):
82+ self.publisher = SnapshotsPublisher()
83+ param = self.parser.parse_args(
84+ ['-t', 'prebuilt', '-j', 'precise-armhf-ubuntu-desktop',
85+ '-n', '1'])
86+ self.publisher.validate_args(param)
87+ build_dir = '/'.join([param.job_name, str(param.build_num)])
88+ build_path = os.path.join(self.uploads_path, build_dir)
89+ os.makedirs(build_path)
90+ www_path = os.path.join(self.target_path, build_dir)
91+ os.makedirs(www_path)
92+
93+ file_name = os.path.join(build_path, buildinfo)
94+ file = open(file_name, "w")
95+ file.close()
96+
97+ self.assertIsNone(self.publisher.check_buildinfo(build_path, www_path))
98+
99+ def test_check_buildinfo_target_dir(self):
100+ self.publisher = SnapshotsPublisher()
101+ param = self.parser.parse_args(
102+ ['-t', 'prebuilt', '-j', 'precise-armhf-ubuntu-desktop',
103+ '-n', '1'])
104+ self.publisher.validate_args(param)
105+ build_dir = '/'.join([param.job_name, str(param.build_num)])
106+ build_path = os.path.join(self.uploads_path, build_dir)
107+ os.makedirs(build_path)
108+ www_path = os.path.join(self.target_path, build_dir)
109+ os.makedirs(www_path)
110+
111+ file_name = os.path.join(www_path, buildinfo)
112+ file = open(file_name, "w")
113+ file.close()
114+
115+ self.assertIsNone(self.publisher.check_buildinfo(build_path, www_path))
116+
117+ def test_check_buildinfo_no_file(self):
118+ self.publisher = SnapshotsPublisher()
119+ param = self.parser.parse_args(
120+ ['-t', 'prebuilt', '-j', 'precise-armhf-ubuntu-desktop',
121+ '-n', '1'])
122+ self.publisher.validate_args(param)
123+ build_dir = '/'.join([param.job_name, str(param.build_num)])
124+ build_path = os.path.join(self.uploads_path, build_dir)
125+ os.makedirs(build_path)
126+ www_path = os.path.join(self.target_path, build_dir)
127+ os.makedirs(www_path)
128+
129+ self.assertRaises(
130+ BuildInfoException,
131+ self.publisher.check_buildinfo, build_path, www_path)

Subscribers

People subscribed via source and target branches