Closed Bug 1083722 Opened 5 years ago Closed 5 years ago

Add an option to jit-tests to ignore timeouts for specified tests

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla36

People

(Reporter: jonco, Assigned: jonco)

Details

Attachments

(1 file)

Running the jit-tests with compacting GC and zeal enabled produces a handful of test timesouts.  This is to be expected as constantly relocating everything in memory will make the tests much slower.

So I'd like to have an option to ignore timeouts from a specified set of tests.  I don't want to just skip those tests as it's still good to get some coverage and make sure they don't fail in some other way.

Here's a patch to add an option --ignore-timeouts=FILE that does this.
Attachment #8506047 - Flags: review?(sphink)
Comment on attachment 8506047 [details] [diff] [review]
jit-test-ignore-timeout-option

Review of attachment 8506047 [details] [diff] [review]:
-----------------------------------------------------------------

This is fine, though it makes me wonder if it should instead be a mapping of files to settings, including gczeal settings, so that you could run these with JS_GC_ZEAL=14,1000 or something and prevent them from timing out that way.

This is simpler for now, though. Shipit.

::: js/src/jit-test/jit_test.py
@@ +218,5 @@
> +    if options.ignore_timeouts:
> +        read_all = False
> +        try:
> +            with open(options.ignore_timeouts) as f:
> +                options.ignore_timeouts = map(lambda line: line.strip('\n'), f.readlines())

wrap that in a set()?

Personally, I'd use

  options.ignore_timeouts = set([line.strip('\n') for line in f.readlines()])

because I find it hard to parse lambdas in argument lists, but either is fine.

@@ +222,5 @@
> +                options.ignore_timeouts = map(lambda line: line.strip('\n'), f.readlines())
> +        except IOError:
> +            sys.exit("Error reading file: " + options.ignore_timeouts)
> +    else:
> +        options.ignore_timeouts = []

set() if you switch to sets above.
Attachment #8506047 - Flags: review?(sphink) → review+
https://hg.mozilla.org/mozilla-central/rev/e7f156e25252
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla36
You need to log in before you can comment on or make changes to this bug.