1487662 - Refactor OOMTest and related test functions to reduce repeated code

Status: RESOLVED FIXED

(Core :: JavaScript Engine, enhancement)

Description

[Jon Coppeard (:jonco)]

The test functions OOMTest, StackOOMTest and InterruptTest share a ton of code.  We should refactor this.

Comment 1

[Jon Coppeard (:jonco)]

Attachment: bug1487662-refactor-oom-tests

I don't know if there's an existing name for this style of testing so I called this 'iterative failure testing'.

The patch factors out the core test algorithm into RunIterativeFailureTest() and the argument parsing into ParseIterativeFailureTestParams().  The test function takes an instance of IterativeFailureSimulator which provides virtual methods to start/stop simulating failures.

Comment 2

[Nicolas B. Pierron [:nbp]]

Comment on attachment 9005602 [details] [diff] [review]
bug1487662-refactor-oom-tests

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

::: js/src/builtin/TestingFunctions.cpp
@@ +1692,5 @@
>  }
>  
> +// Iterative failure testing: test what happens when a certain kind of failure
> +// occurs at every possible point. For example, trigger OOM at every allocation
> +// point and test that it is handled correctly.

nit: Stress test a function calls, by simulating failures at predefined indexed locations across the normal execution path, and checks that the remaining state of the environment is consistent with the returned error code.

@@ +1747,4 @@
>              fprintf(stderr, "thread %d\n", thread);
>  
>          unsigned allocation = 1;
>          bool handledOOM;

nit: rename `allocation` to `iteration`, and `handledOOM` to `next`. Do the same renaming in fprintf strings too.

Comment 3

[Jon Coppeard (:jonco)]

(In reply to Nicolas B. Pierron [:nbp] from comment #2)
> nit: Stress test a function calls, by simulating failures at predefined
> indexed locations across the normal execution path, and checks that the
> remaining state of the environment is consistent with the returned error
> code.

I don't think that this should be called stress testing, but I'll incorporate the other bits.

> nit: rename `allocation` to `iteration`, and `handledOOM` to `next`. Do the
> same renaming in fprintf strings too.

Thanks, I was going to do that and forgot.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/29f80c699b14
Refactor OOMTest and related functions r=nbp

Comment 5

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/29f80c699b14