[Compile perf] Use lit tmpdir in scale-test, rdar://29090287 #5675
Conversation
| with open(ifile,'w+') as f: | ||
| fname = "in%d.swift" % n | ||
| pathname = os.path.join(d, fname) | ||
| with open(pathname,'w+') as f: |
There was a problem hiding this comment.
nit-pick: You should run the flake8 linter over this code. I think this line will produce a warning, because it's missing a space after the comma.
There was a problem hiding this comment.
Interesting. I see the results in the jenkins job, but when I install and run flake8 locally I get no errors. Is there some particular version or configuration of flake8 to install to get such results?
| r = {} | ||
| try: | ||
| d = tempfile.mkdtemp() | ||
| if args.tmpdir != None and not os.path.exists(args.tmpdir): |
There was a problem hiding this comment.
nit-pick: The linter will probably complain here as well. None is a singleton, the convention is to check for identity, not equality. So this should be if args.tmpdir is not None.
|
@swift-ci Please Python lint |
| @@ -269,6 +275,8 @@ def main(): | |||
|
|
|||
| args = parser.parse_args(sys.argv[1:]) | |||
| (rng, runs) = run_many(args) | |||
| if args.tmpdir != None: | |||
| shutil.rmtree(args.tmpdir) | |||
There was a problem hiding this comment.
Is there any reason we have to delete this at all? If there is, can we do it before the test instead of after? Otherwise we delete valuable artifacts for debugging failures.
There was a problem hiding this comment.
No strong reason, just thought I'd clean up. Will move to before-test.
There was a problem hiding this comment.
Eh, on second thought some users might not expect a tmpdir to be deleted before use, I'll just remove these lines.
There was a problem hiding this comment.
Cool. LGTM with that change!
graydon
force-pushed
the
rdar-29090287-failing-scale-test-tmpdir
branch
from
6b30fb2 to
2a89323
Compare
|
Updated with flake8 fixes and removal of rmtree call. |
|
@swift-ci please smoke test and merge |
This PR makes the
scale-testscript andlit.cfgconfigury a little more precise in its use of paths, to try to work around some CI failures we were seeing. Changes are:swiftcpath rather than searching $PATH%ttest tempdir rather than letting python'stempfilesearch for oneThe second change is likely the one that was causing the CI failure (
lit.pyscrubs the environment of the subprocess, meaningscale-testwas putting files in/tmp/...not/var/folders/...) but the first and third changes here improve the precision and reduce the chatter of the subprocess along the way.The CI breakage is tracked in rdar://29090287