Closed Bug 1487662 Opened Last year Closed Last year

Refactor OOMTest and related test functions to reduce repeated code

Categories

(Core :: JavaScript Engine, enhancement)

61 Branch
enhancement
Not set

Tracking

()

RESOLVED FIXED
mozilla63
Tracking Status
firefox63 --- fixed

People

(Reporter: jonco, Assigned: jonco)

References

Details

Attachments

(1 file)

The test functions OOMTest, StackOOMTest and InterruptTest share a ton of code.  We should refactor this.
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.
Attachment #9005602 - Flags: review?(nicolas.b.pierron)
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.
Attachment #9005602 - Flags: review?(nicolas.b.pierron) → review+
(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.
Pushed by jcoppeard@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/29f80c699b14
Refactor OOMTest and related functions r=nbp
https://hg.mozilla.org/mozilla-central/rev/29f80c699b14
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in before you can comment on or make changes to this bug.