Last Comment Bug 650629 - Clean-up and fix tests landed with bug 583514
: Clean-up and fix tests landed with bug 583514
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Core & HTML (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla6
Assigned To: Mounir Lamouri (:mounir)
:
: Andrew Overholt [:overholt]
Mentors:
Depends on:
Blocks: 583514 650602
  Show dependency treegraph
 
Reported: 2011-04-17 06:14 PDT by Mounir Lamouri (:mounir)
Modified: 2011-04-18 04:51 PDT (History)
3 users (show)
mounir: in‑testsuite+
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Cleaning (8.22 KB, patch)
2011-04-17 06:58 PDT, Mounir Lamouri (:mounir)
ehsan: review+
Details | Diff | Splinter Review

Description Mounir Lamouri (:mounir) 2011-04-17 06:14:36 PDT
Tests from bug 583514 contain a lot of issues:
- Reftests 583514-1.html and 583514-2.html are wrong because they do not use class="reftest-wait" creating an intermittent failure;
- The mochitest isn't added to Makefile.in so it's never run;
- The mochitest is using netscape.security.PrivilegeManager.enablePrivilege which has been explicitly forbidden some time ago;
- The mochitest is using events but do not use event handlers (a trap for random oranges);
- It is adding a reftest with a bug number that doesn't correspond to the test bug (bug 409604);
- Two reftests are doing exactly the same thing: 409604-1.html and 583514-2.html but one of them don't use reftest-wait;
- All reftests would be much more easier to write with mochitests...
Comment 1 Mounir Lamouri (:mounir) 2011-04-17 06:58:21 PDT
Created attachment 526582 [details] [diff] [review]
Cleaning

All reftests have been removed in favor of a single mochitest.
The original mochitest has been removed because content/html/content/test/test_bug409604.html seems to cover what was tested in it.
Comment 2 :Ehsan Akhgari 2011-04-17 14:42:39 PDT
(In reply to comment #0)
> Tests from bug 583514 contain a lot of issues:
> - Reftests 583514-1.html and 583514-2.html are wrong because they do not use
> class="reftest-wait" creating an intermittent failure;
> - The mochitest isn't added to Makefile.in so it's never run;
> - The mochitest is using netscape.security.PrivilegeManager.enablePrivilege
> which has been explicitly forbidden some time ago;
> - The mochitest is using events but do not use event handlers (a trap for
> random oranges);
> - It is adding a reftest with a bug number that doesn't correspond to the test
> bug (bug 409604);
> - Two reftests are doing exactly the same thing: 409604-1.html and
> 583514-2.html but one of them don't use reftest-wait;
> - All reftests would be much more easier to write with mochitests...

Thanks for catching this.  All of these issues should have ideally been caught at review time.  :(
Comment 3 Mounir Lamouri (:mounir) 2011-04-18 04:51:14 PDT
Pushed:
http://hg.mozilla.org/mozilla-central/rev/189388b079a8

Note You need to log in before you can comment on or make changes to this bug.