Closed Bug 1277770 Opened 3 years ago Closed 3 years ago

Make the jit-test runner look for a per-directory directive file

Categories

(Core :: JavaScript Engine: JIT, defect, P2)

defect

Tracking

()

RESOLVED FIXED
mozilla52
Tracking Status
firefox52 --- fixed

People

(Reporter: lth, Assigned: lth)

References

Details

Attachments

(1 file)

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.)
Priority: -- → P3
Priority: P3 → P5
Blocks: 1313114
P2 to avoid a priority inversion.
Priority: P5 → P2
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)
BTW, examples of how this is used will appear shortly on bug 1313114.
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+
https://hg.mozilla.org/mozilla-central/rev/788c788561b4
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla52
You need to log in before you can comment on or make changes to this bug.