Closed
Bug 1277770
Opened 8 years ago
Closed 8 years ago
Make the jit-test runner look for a per-directory directive file
Categories
(Core :: JavaScript Engine: JIT, defect, P2)
Core
JavaScript Engine: JIT
Tracking
()
RESOLVED
FIXED
mozilla52
Tracking | Status | |
---|---|---|
firefox52 | --- | fixed |
People
(Reporter: lth, Assigned: lth)
References
Details
Attachments
(1 file)
2.35 KB,
patch
|
jonco
:
review+
|
Details | Diff | Splinter Review |
Spun out as future work from https://bugzilla.mozilla.org/show_bug.cgi?id=1232205#c33 and previous comments: We would like the test runner to have the option of not just scanning a file for directives in its header (ie, "|jit-test|" ...) but for it to recognize a per-directory file containing such directives, a strawman name for that file is directives.txt. The meaning of the file would be that the directives in the file would be merged with the directives in each file in the directory. (I'm on the fence regarding the file applying to subdirectories, I'm thinking probably not.)
Updated•8 years ago
|
Priority: -- → P3
Assignee | ||
Updated•8 years ago
|
Priority: P3 → P5
Assignee | ||
Comment 2•8 years ago
|
||
Looks for directives.txt in the directory of the test case, and extracts a jit-test directive out of that if there's one. It is then catenated with whatever's in the test file. The directive is cached in the test runner, though I don't know if that really is necessary.
Attachment #8808575 -
Flags: review?(jcoppeard)
Assignee | ||
Comment 3•8 years ago
|
||
BTW, examples of how this is used will appear shortly on bug 1313114.
Comment 4•8 years ago
|
||
Comment on attachment 8808575 [details] [diff] [review] bug1277770-jit-test-directives-from-file.patch Review of attachment 8808575 [details] [diff] [review]: ----------------------------------------------------------------- This looks good. Maybe you should check if there are extra lines in the directives file that will get ignored though. ::: js/src/tests/lib/jittests.py @@ +180,5 @@ > + # the first line is considered. > + > + dir_name = os.path.dirname(path) > + if not dir_name.endswith('/'): > + dir_name += '/' You don't need to add the slash here... @@ +186,5 @@ > + dir_meta = '' > + if dir_name in cls.Directives: > + dir_meta = cls.Directives[dir_name] > + else: > + meta_file_name = dir_name + "directives.txt" ...if you use os.path.join here. @@ +191,5 @@ > + if os.path.exists(meta_file_name): > + line = open(meta_file_name).readline() > + i = line.find(cls.COOKIE) > + if i != -1: > + dir_meta = ';' + line[i + len(cls.COOKIE):].strip('\n') Please factor out this parsing code into a separate function to avoid repeating it below. Also it's probably more readable to call split() on the string than using find() and subscript to do this.
Attachment #8808575 -
Flags: review?(jcoppeard) → review+
Assignee | ||
Comment 5•8 years ago
|
||
Try run with this and other patches: https://treeherder.mozilla.org/#/jobs?repo=try&revision=193cdd4713be7528e5de69c0439c3213cb05116d
Assignee | ||
Comment 6•8 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/788c788561b43fc079572b32d75e7f2f1f13cd41 Bug 1277770 - look for jit-test directives in directives.txt. r=jonco
Comment 7•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/788c788561b4
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox52:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in
before you can comment on or make changes to this bug.
Description
•