Merge lp:~deeptik/linaro-license-protection/publish-to-snapshot into lp:~linaro-automation/linaro-license-protection/trunk
- publish-to-snapshot
- Merge into trunk
Status: | Merged |
---|---|
Merged at revision: | 47 |
Proposed branch: | lp:~deeptik/linaro-license-protection/publish-to-snapshot |
Merge into: | lp:~linaro-automation/linaro-license-protection/trunk |
Diff against target: |
354 lines (+330/-1) 3 files modified
scripts/publish_to_snapshots.py (+142/-0) testing/__init__.py (+5/-1) testing/test_publish_to_snapshots.py (+183/-0) |
To merge this branch: | bzr merge lp:~deeptik/linaro-license-protection/publish-to-snapshot |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Paul Sokolovsky | Approve | ||
Review via email: mp+96735@code.launchpad.net |
Commit message
Description of the change
Script to move the artifacts from tmp location to permanent location on s.l.o.
I have retained the example uploads and target dir in the script so that it would be easy for you to test the code.
Here is the ex: inputs you can use to verify the script
1) For Android
$mkdir -p /tmp/uploads/
$scripts/
Moved the files from /tmp/uploads/
Move succeeded
2) For kernel-hwpack
$ mkdir -p /tmp/uploads/
$ scripts/
Moved the files from /tmp/uploads/
Move succeeded
Paul Sokolovsky (pfalcon) wrote : | # |
" help="Specifiy the job type (Ex: android/
Typo in "specify". Also, would suggest to rewrite to avoid possible confusion, e.g.:
"Specify the job type (Values: android, kernel-hwpack)"
Paul Sokolovsky (pfalcon) wrote : | # |
9
parser.
10
11
12
13
This seems like confusing option. Reading the help, I don't have an idea why it is called "archive_info". Nor reading help I have clear picture what and how it should be used/works - that's too much specifics, out of context. This script is supposed to be called from Jenkins, and Jenkins cab basically pass to it it job name (in its native format) and build number. That's what script should accept, and help for options should describe just that, any specifics like the fact that job name have internal structure based on --job-type should be left to manual IMHO.
- 46. By Georgy Redkozubov
-
[merge] New STE license text
Paul Sokolovsky (pfalcon) wrote : | # |
Also, short option assignment is not ideal IMHO. For "job type", main word is (t)ype. For "kernel tree" it's (k)ernel (or (r)epository if should be generalized), "build number" is apparently (n)umber, which leaves "job name" to (j)ob.
Paul Sokolovsky (pfalcon) wrote : | # |
60 + if args.job_type == "android":
61 + build_path = '/'.join(
Well, nope, that's not correct. There's no "user name" on Jenkins level, only job name. That's what this script accepts. It then can reformat native Jenkins name into "external" name which we want to see.
Paul Sokolovsky (pfalcon) wrote : | # |
71 + if not (os.path.
72 + print "Missing build path", build_dir_path
73 + return FAIL
The error message is incorrect - it checks 2 dirs, but reports problem as if applies to only one. Such cases are always great source of confusion and suffering (the latest one, (c) by Google: https:/
Paul Sokolovsky (pfalcon) wrote : | # |
79 + try:
80 + # Make a note of the contents of src dir so that
81 + # it can be used to validate the move to destination
82 + uploads_dirList = os.listdir(
That sounds complicated and overinsuring. Can't we make requirement that upload dir and destination dir were on the same FS, so we can just use mv? Even if we can't, why wouldn't we trust shutil.copy2()? It either does the copy, or throws an exception, so we can't miss failed move.
Minor note - would be nice to use consistent var naming convention, like uploads_dir_list.
Paul Sokolovsky (pfalcon) wrote : | # |
111 +def main():
112 + uploads_path = '/srv3/
113 + target_path = '/srv3/
114 + uploads_path = '/tmp/uploads/'
115 + target_path = '/tmp/www/'
IMHO, it would be better to treat these as the constants, not as the variables, to be defined in the beginning of the script (like they were before r50).
Deepti B. Kalakeri (deeptik) wrote : | # |
Thanks paul for comments. I will reply back on tuesday when am back to office.
Thanks!!!
Deepti
On 3/9/12, Paul Sokolovsky <email address hidden> wrote:
> 111 +def main():
> 112 + uploads_path = '/srv3/
> 113 + target_path = '/srv3/
> 114 + uploads_path = '/tmp/uploads/'
> 115 + target_path = '/tmp/www/'
>
> IMHO, it would be better to treat these as the constants, not as the
> variables, to be defined in the beginning of the script (like they were
> before r50).
>
> --
> https:/
> You are the owner of
> lp:~deeptik/linaro-license-protection/publish-to-snapshot.
>
--
Sent from my mobile device
Thanks and Regards,
Deepti
Infrastructure Team Member, Linaro Platform Teams
Linaro.org | Open source software for ARM SoCs
Follow Linaro: http://
http://
Deepti B. Kalakeri (deeptik) wrote : | # |
> " help="Specifiy the job type (Ex: android/
>
> Typo in "specify". Also, would suggest to rewrite to avoid possible confusion,
> e.g.:
> "Specify the job type (Values: android, kernel-hwpack)"
oops! copy paste effect.. done.
Thanks!!!
Deepti.
Deepti B. Kalakeri (deeptik) wrote : | # |
>
> 9
> parser.
> 10
> help="Specify the job which resulted the archive to be
> stored.\
> 11
> Ex: ${JOB_NAME}
> for \
> 12
> andriod and for \
> 13
> kernel-hwpack ${KERNEL_
>
> This seems like confusing option. Reading the help, I don't have an idea why
> it is called "archive_info". Nor reading help I have clear picture what and
> how it should be used/works - that's too much specifics, out of context. This
> script is supposed to be called from Jenkins, and Jenkins cab basically pass
> to it it job name (in its native format) and build number. That's what script
> should accept, and help for options should describe just that, any specifics
> like the fact that job name have internal structure based on --job-type should
> be left to manual IMHO.
There is not archive_info anymore. This was an old change.
> Also, short option assignment is not ideal IMHO. For "job type", main word is
> (t)ype. For "kernel tree" it's (k)ernel (or (r)epository if should be
> generalized), "build number" is apparently (n)umber, which leaves "job name"
> to (j)ob.
Good suggestions. done.
> 60 + if args.job_type == "android":
> 61 + build_path = '/'.join(
> args.job_name])
>
> Well, nope, that's not correct. There's no "user name" on Jenkins level, only
> job name. That's what this script accepts. It then can reformat native Jenkins
> name into "external" name which we want to see.
Yes I agree that there is no username at jenkins level. But for android, I believe the jenkins-post-www.sh script line 38 refer to $TARGET_
So I just tried to retain the extra work of separating the jobname into username + jobname.
Let me know if this is not required and what exactly did the username in the script meant to be.
I will make the changes accordingly.
> 71 + if not (os.path.
> os.path.
> 72 + print "Missing build path", build_dir_path
> 73 + return FAIL
>
> The error message is incorrect - it checks 2 dirs, but reports problem as if
> applies to only one. Such cases are always great source of confusion and
> suffering (the latest one, (c) by Google:
> https:/
I agree. Modified this to correct the error message.
> 79 + try:
> 80 + # Make a note of the contents of src dir so that
> 81 + # it can be used to validate the move to destination
> 82 + uploads_dirList = os.listdir(
>
> That sounds complicated and overinsuring. Can't we make requirement that
> upload dir and destination dir were on the same FS, so we can just use mv?
> Even if we can't, why wouldn't we trust shutil.copy2()? It either does the
> copy, or throws an exception, so we can't miss failed move.
>
I did consider using the mv action for this. shutil.move has the following issue:
1) The destination dir should be non-existent. so in case of kernel hwpacks we intend to store all the hwpack...
Paul Sokolovsky (pfalcon) wrote : | # |
On Tue, 13 Mar 2012 09:55:27 -0000
"Deepti B. Kalakeri" <email address hidden> wrote:
> >
> > 9
> > parser.
> > 10
> > help="Specify the job which resulted the
> > archive to be stored.\
> > 11
> > Ex: ${JOB_NAME}
> > specified for \
> > 12
> > andriod and for \
> > 13
> > kernel-hwpack
> > ${KERNEL_
> >
> > This seems like confusing option. Reading the help, I don't have an
> > idea why it is called "archive_info". Nor reading help I have clear
> > picture what and how it should be used/works - that's too much
> > specifics, out of context. This script is supposed to be called
> > from Jenkins, and Jenkins cab basically pass to it it job name (in
> > its native format) and build number. That's what script should
> > accept, and help for options should describe just that, any
> > specifics like the fact that job name have internal structure based
> > on --job-type should be left to manual IMHO.
>
> There is not archive_info anymore. This was an old change.
>
> > Also, short option assignment is not ideal IMHO. For "job type",
> > main word is (t)ype. For "kernel tree" it's (k)ernel (or
> > (r)epository if should be generalized), "build number" is
> > apparently (n)umber, which leaves "job name" to (j)ob.
>
> Good suggestions. done.
>
> > 60 + if args.job_type == "android":
> > 61 + build_path = '/'.join(
> > args.job_name])
> >
> > Well, nope, that's not correct. There's no "user name" on Jenkins
> > level, only job name. That's what this script accepts. It then can
> > reformat native Jenkins name into "external" name which we want to
> > see.
>
> Yes I agree that there is no username at jenkins level. But for
> android, I believe the jenkins-post-www.sh script line 38 refer to
> $TARGET_
Yes, and those are parsed from Jenkin job name in line 32 and on:
http://
> So I just tried to retain the extra
> work of separating the jobname into username + jobname. Let me know
> if this is not required and what exactly did the username in the
> script meant to be. I will make the changes accordingly.
So, the script should accept ${JOB_NAME} from Jenkins (we simply
don't have anything more specific on its level), but then it should be
ready to parse/convert that into job-type-specific target path (i.e.
this conversion will be different for different job-type). So, I would
suggest to add job_to_
encapsulate this conversion. For android case, it will be equivalent of
lines 32-38 in jenkins-post-www.sh .
>
> > 71 + if not (os.path.
> > os.path.
> > 72 + print "Missing build path", build_dir_path
> > 73 + return FAIL
> >
> > The error message is incorrect - it checks 2 dirs, but reports
> > problem as if applies to only one. Such cases are alway...
Paul Sokolovsky (pfalcon) wrote : | # |
Ok, based on the further discussion on IRC, this should be good enough to proceed, further elaboration can be done separately.
- 47. By Deepti B. Kalakeri
-
Script to move the artifacts from tmp location to permanent location on s.l.o.
Preview Diff
1 | === added file 'scripts/__init__.py' |
2 | === added file 'scripts/publish_to_snapshots.py' |
3 | --- scripts/publish_to_snapshots.py 1970-01-01 00:00:00 +0000 |
4 | +++ scripts/publish_to_snapshots.py 2012-03-14 08:03:18 +0000 |
5 | @@ -0,0 +1,142 @@ |
6 | +#!/usr/bin/env python |
7 | +# This script moves the artifacts from tmp location to a |
8 | +# to a permanent location on s.l.o |
9 | + |
10 | +import os |
11 | +import sys |
12 | +import shutil |
13 | +import argparse |
14 | + |
15 | +parser = argparse.ArgumentParser() |
16 | +parser.add_argument("-t", "--job-type", dest="job_type", |
17 | + help="Specify the job type (Ex: android/kernel-hwpack)") |
18 | +parser.add_argument("-r", "--ktree", dest="kernel_tree", |
19 | + help="Specify the kernel tree built by the job") |
20 | +parser.add_argument("-j", "--job-name", dest="job_name", |
21 | + help="Specify the job name which resulted the archive to be stored.\ |
22 | + Ex: ${JOB_NAME} should be specified for andriod and for \ |
23 | + kernel-hwpack ${KERNEL_JOB_NAME}") |
24 | +parser.add_argument("-n", "--build-num", dest="build_num", type=int, |
25 | + help="Specify the job build number for android builds only") |
26 | + |
27 | +uploads_path = '/srv3/snapshots.linaro.org/uploads/' |
28 | +target_path = '/srv3/snapshots.linaro.org/www/' |
29 | +PASS = 0 |
30 | +FAIL = 1 |
31 | +acceptable_job_types = [ |
32 | + 'android', |
33 | + 'kernel-hwpack', |
34 | + ] |
35 | + |
36 | +class SnapshotsPublisher(object): |
37 | + |
38 | + def validate_args(self, args): |
39 | + # Validate that all the required information |
40 | + # is passed on the command line |
41 | + if (args.job_type == None or args.job_name == None): |
42 | + parser.error("\nYou must specify job-type and job-name") |
43 | + return FAIL |
44 | + |
45 | + if (args.job_type == "android" and args.build_num == None): |
46 | + parser.error("You must specify build number") |
47 | + return FAIL |
48 | + |
49 | + if (args.job_type == "kernel-hwpack" and args.kernel_tree == None): |
50 | + parser.error("You must specify kernel tree name built by the job") |
51 | + return FAIL |
52 | + |
53 | + if (args.job_type not in acceptable_job_types): |
54 | + parser.error("Invalid job type") |
55 | + return FAIL |
56 | + |
57 | + def jobname_to_target_subdir(self, args, jobname): |
58 | + ret_val = None |
59 | + if args.job_type == "android": |
60 | + ret_val = jobname.split("_") |
61 | + return ret_val |
62 | + |
63 | + def validate_paths(self, args, uploads_path, target_path): |
64 | + build_dir_path = target_dir_path = None |
65 | + if args.job_type == "android": |
66 | + build_path = '/'.join([args.job_type, args.job_name, |
67 | + str(args.build_num)]) |
68 | + build_dir_path = os.path.join(uploads_path, build_path) |
69 | + ret_val = self.jobname_to_target_subdir(args, args.job_name) |
70 | + if ret_val != None: |
71 | + user_name = ret_val[0] |
72 | + job_name = '_'.join(ret_val[1:]) |
73 | + target_dir = '/'.join([args.job_type, user_name, job_name, |
74 | + str(args.build_num)]) |
75 | + target_dir_path = os.path.join(target_path, target_dir) |
76 | + else: |
77 | + return None, None |
78 | + else: |
79 | + build_path = '/'.join([args.job_type, args.kernel_tree, args.job_name]) |
80 | + build_dir_path = os.path.join(uploads_path, build_path) |
81 | + target_dir_path = os.path.join(target_path, build_path) |
82 | + |
83 | + if not (os.path.isdir(uploads_path) or os.path.isdir(build_dir_path)): |
84 | + build_paths = "'%s' or '%s'" % (uploads_path, build_dir_path) |
85 | + print "Missing build paths: ", build_paths |
86 | + return None, None |
87 | + |
88 | + if not os.path.isdir(target_path): |
89 | + print "Missing target path", target_path |
90 | + return None, None |
91 | + |
92 | + return build_dir_path, target_dir_path |
93 | + |
94 | + |
95 | + def move_artifacts(self, args, build_dir_path, target_dir_path): |
96 | + try: |
97 | + # Make a note of the contents of src dir so that |
98 | + # it can be used to validate the move to destination |
99 | + uploads_dir_list = os.listdir(build_dir_path) |
100 | + |
101 | + if not os.path.isdir(target_dir_path): |
102 | + os.makedirs(target_dir_path) |
103 | + if not os.path.isdir(target_dir_path): |
104 | + raise OSError |
105 | + |
106 | + for fname in uploads_dir_list: |
107 | + fname = os.path.join(build_dir_path, fname) |
108 | + shutil.copy2(fname, target_dir_path) |
109 | + |
110 | + target_dir_list = os.listdir(target_dir_path) |
111 | + for fname in uploads_dir_list: |
112 | + if not fname in target_dir_list: |
113 | + print "Destination missing file", fname |
114 | + return FAIL |
115 | + |
116 | + shutil.rmtree(build_dir_path) |
117 | + print "Moved the files from '",build_dir_path, "' to '",\ |
118 | + target_dir_path, "'" |
119 | + return PASS |
120 | + |
121 | + except OSError, details: |
122 | + print "Failed to create the target path", target_dir_path, ":" , details |
123 | + return FAIL |
124 | + except shutil.Error: |
125 | + print "Failed to move files destination path", target_dir_path |
126 | + print "Target already exists, move failed" |
127 | + return FAIL |
128 | + |
129 | +def main(): |
130 | + publisher = SnapshotsPublisher() |
131 | + args = parser.parse_args() |
132 | + publisher.validate_args(args) |
133 | + build_dir_path, target_dir_path = publisher.validate_paths(args, uploads_path, |
134 | + target_path) |
135 | + if build_dir_path == None or target_dir_path == None: |
136 | + print "Problem with build/target path, move failed" |
137 | + return FAIL |
138 | + |
139 | + ret = publisher.move_artifacts(args, build_dir_path, target_dir_path) |
140 | + if ret != PASS: |
141 | + print "Move Failed" |
142 | + return FAIL |
143 | + else: |
144 | + print "Move succeeded" |
145 | + return PASS |
146 | +if __name__ == '__main__': |
147 | + sys.exit(main()) |
148 | |
149 | === modified file 'testing/__init__.py' |
150 | --- testing/__init__.py 2012-01-12 14:07:05 +0000 |
151 | +++ testing/__init__.py 2012-03-14 08:03:18 +0000 |
152 | @@ -1,9 +1,13 @@ |
153 | import os |
154 | import unittest |
155 | |
156 | +from testing.test_click_through_license import * |
157 | +from testing.test_publish_to_snapshots import * |
158 | + |
159 | def test_suite(): |
160 | module_names = [ |
161 | - 'testing.test_click_through_license', |
162 | + 'testing.test_click_through_license.TestLicense', |
163 | + 'testing.test_publish_to_snapshots.TestSnapshotsPublisher', |
164 | ] |
165 | loader = unittest.TestLoader() |
166 | suite = loader.loadTestsFromNames(module_names) |
167 | |
168 | === added file 'testing/test_publish_to_snapshots.py' |
169 | --- testing/test_publish_to_snapshots.py 1970-01-01 00:00:00 +0000 |
170 | +++ testing/test_publish_to_snapshots.py 2012-03-14 08:03:18 +0000 |
171 | @@ -0,0 +1,183 @@ |
172 | +#!/usr/bin/env python |
173 | + |
174 | +import os |
175 | +import sys |
176 | +import shutil |
177 | +import tempfile |
178 | +import argparse |
179 | +from StringIO import StringIO |
180 | +from testtools import TestCase |
181 | +from scripts.publish_to_snapshots import SnapshotsPublisher |
182 | + |
183 | +class TestSnapshotsPublisher(TestCase): |
184 | + '''Tests for publishing files to the snapshots.l.o www are.''' |
185 | + |
186 | + uploads_path = "./uploads/" |
187 | + target_path = "./www/" |
188 | + |
189 | + def setUp(self): |
190 | + self.parser = argparse.ArgumentParser() |
191 | + self.parser.add_argument("-t", "--job-type", dest="job_type") |
192 | + self.parser.add_argument("-r", "--ktree", dest="kernel_tree") |
193 | + self.parser.add_argument("-j", "--job-name", dest="job_name") |
194 | + self.parser.add_argument("-n", "--build-num", dest="build_num", type=int) |
195 | + if not os.path.isdir(self.uploads_path): |
196 | + os.mkdir(self.uploads_path) |
197 | + |
198 | + if not os.path.isdir(self.target_path): |
199 | + os.mkdir(self.target_path) |
200 | + super(TestSnapshotsPublisher, self).setUp() |
201 | + |
202 | + def tearDown(self): |
203 | + if os.path.isdir(self.uploads_path): |
204 | + shutil.rmtree(self.uploads_path) |
205 | + |
206 | + if os.path.isdir(self.target_path): |
207 | + shutil.rmtree(self.target_path) |
208 | + super(TestSnapshotsPublisher, self).tearDown() |
209 | + |
210 | + def test_validate_args_valid_job_values(self): |
211 | + self.publisher = SnapshotsPublisher() |
212 | + param = self.parser.parse_args(['-t', 'android', '-j', 'dummy_job_name', '-n', '1']) |
213 | + self.publisher.validate_args(param) |
214 | + param = self.parser.parse_args(['-t', 'kernel-hwpack', '-r', 'dummy_tree', |
215 | + '-j', 'dummy_job_name']) |
216 | + self.publisher.validate_args(param) |
217 | + |
218 | + def test_validate_args_invalid_job_type(self): |
219 | + orig_stderr = sys.stderr |
220 | + stderr = sys.stderr = StringIO() |
221 | + self.publisher = SnapshotsPublisher() |
222 | + param = self.parser.parse_args(['-t', 'invalid_job_type', '-j', 'dummy_job_name', |
223 | + '-n', '1']) |
224 | + try: |
225 | + self.publisher.validate_args(param) |
226 | + except SystemExit, err: |
227 | + self.assertEqual(err.code, 2, "Expected result") |
228 | + finally: |
229 | + sys.stderr = orig_stderr |
230 | + stderr.seek(0) |
231 | + self.assertIn("Invalid job type", stderr.read()) |
232 | + |
233 | + |
234 | + def test_validate_args_run_invalid_argument(self): |
235 | + orig_stderr = sys.stderr |
236 | + stderr = sys.stderr = StringIO() |
237 | + self.publisher = SnapshotsPublisher() |
238 | + try: |
239 | + param = self.parser.parse_args(['-a']) |
240 | + self.publisher.validate_args(param) |
241 | + except SystemExit, err: |
242 | + self.assertEqual(err.code, 2, "Invalid argument passed") |
243 | + finally: |
244 | + sys.stderr = orig_stderr |
245 | + stderr.seek(0) |
246 | + self.assertIn("unrecognized arguments: -a\n", stderr.read()) |
247 | + |
248 | + def test_validate_args_run_invalid_value(self): |
249 | + orig_stderr = sys.stderr |
250 | + stderr = sys.stderr = StringIO() |
251 | + self.publisher = SnapshotsPublisher() |
252 | + try: |
253 | + param = self.parser.parse_args(['-n', "N"]) |
254 | + self.publisher.validate_args(param) |
255 | + except SystemExit, err: |
256 | + self.assertEqual(err.code, 2, "Invalid value passed") |
257 | + finally: |
258 | + sys.stderr = orig_stderr |
259 | + stderr.seek(0) |
260 | + self.assertIn("argument -n/--build-num: invalid int value: 'N'", |
261 | + stderr.read()) |
262 | + |
263 | + def test_validate_args_run_none_values(self): |
264 | + orig_stderr = sys.stderr |
265 | + stderr = sys.stderr = StringIO() |
266 | + self.publisher = SnapshotsPublisher() |
267 | + try: |
268 | + param = self.parser.parse_args(['-t', None , '-r', None, |
269 | + '-j', None , '-n' , 0]) |
270 | + self.publisher.validate_args(param) |
271 | + except SystemExit, err: |
272 | + self.assertEqual(err.code, 2, "None values are not acceptable") |
273 | + finally: |
274 | + sys.stderr = orig_stderr |
275 | + stderr.seek(0) |
276 | + self.assertIn("You must specify job-type and job-name", |
277 | + stderr.read()) |
278 | + |
279 | + def test_validate_paths_invalid_uploads_path(self): |
280 | + orig_stdout = sys.stdout |
281 | + stdout = sys.stdout = StringIO() |
282 | + self.publisher = SnapshotsPublisher() |
283 | + param = self.parser.parse_args(['-t', 'android', '-j', 'dummy_job_name', |
284 | + '-n', '1']) |
285 | + |
286 | + self.publisher.validate_args(param) |
287 | + self.uploads_path = "./dummy_uploads_path" |
288 | + try: |
289 | + self.publisher.validate_paths(param, self.uploads_path, self.target_path) |
290 | + finally: |
291 | + sys.stdout = orig_stdout |
292 | + stdout.seek(0) |
293 | + self.assertIn("Missing build path", stdout.read()) |
294 | + |
295 | + def test_validate_paths_invalid_target_path(self): |
296 | + orig_stdout = sys.stdout |
297 | + stdout = sys.stdout = StringIO() |
298 | + self.publisher = SnapshotsPublisher() |
299 | + param = self.parser.parse_args(['-t', 'android', '-j', 'dummy_job_name', |
300 | + '-n', '1']) |
301 | + |
302 | + self.publisher.validate_args(param) |
303 | + self.target_path = "./dummy_target_path" |
304 | + try: |
305 | + self.publisher.validate_paths(param, self.uploads_path, |
306 | + self.target_path) |
307 | + finally: |
308 | + sys.stdout = orig_stdout |
309 | + stdout.seek(0) |
310 | + self.assertIn("Missing target path", stdout.read()) |
311 | + |
312 | + def test_move_artifacts_kernel_successful_move(self): |
313 | + orig_stdout = sys.stdout |
314 | + stdout = sys.stdout = StringIO() |
315 | + self.publisher = SnapshotsPublisher() |
316 | + param = self.parser.parse_args(['-t', 'kernel-hwpack', '-j', 'dummy_job_name', |
317 | + '-r', 'dummy_kernel_tree']) |
318 | + self.publisher.validate_args(param) |
319 | + build_path = os.path.join(self.uploads_path, param.job_type, param.kernel_tree, |
320 | + param.job_name) |
321 | + os.makedirs(build_path) |
322 | + tempfiles = tempfile.mkstemp(dir=build_path) |
323 | + try: |
324 | + uploads_dir_path, target_dir_path = self.publisher.validate_paths(param, |
325 | + self.uploads_path, self.target_path) |
326 | + self.publisher.move_artifacts(param, uploads_dir_path, target_dir_path) |
327 | + finally: |
328 | + sys.stdout = orig_stdout |
329 | + pass |
330 | + |
331 | + stdout.seek(0) |
332 | + self.assertIn("Moved the files from", stdout.read()) |
333 | + |
334 | + def test_move_artifacts_android_successful_move(self): |
335 | + orig_stdout = sys.stdout |
336 | + stdout = sys.stdout = StringIO() |
337 | + self.publisher = SnapshotsPublisher() |
338 | + param = self.parser.parse_args(['-t', 'android', '-j', 'dummy_job_name', |
339 | + '-n', '1']) |
340 | + self.publisher.validate_args(param) |
341 | + build_dir = '/'.join([param.job_type, param.job_name, str(param.build_num)]) |
342 | + build_path = os.path.join(self.uploads_path, build_dir) |
343 | + os.makedirs(build_path) |
344 | + tempfiles = tempfile.mkstemp(dir=build_path) |
345 | + try: |
346 | + uploads_dir_path, target_dir_path = self.publisher.validate_paths(param, |
347 | + self.uploads_path, self.target_path) |
348 | + self.publisher.move_artifacts(param, uploads_dir_path, target_dir_path) |
349 | + finally: |
350 | + sys.stdout = orig_stdout |
351 | + pass |
352 | + |
353 | + stdout.seek(0) |
354 | + self.assertIn("Moved the files from", stdout.read()) |
One immediate comment is inconsistent format of the long options, like "--job_type". It's GNU (originators of long options) convention that long options use dashes as separators. It's of course also looks better and easier to remember. We had/have the same issue with l-m-c: there was non-compliant options, then there was a patch to fix it, then it turned out it breaks existing scripts which pass those options, then the fix had to be reverted or something, etc. So, we should do it rightly from the start.