642704 - Integrate the BrowserScope richtext2 test suite into our automated testing framework

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[(no longer active)]

I'm filing this bug to track the addition of the richtext2 test suite into our mochitest suite.

Comment 1

[(no longer active)]

Attachment: Part 1: Import the richtext2 test suite into the tree

Comment 2

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Gerv, is it OK to have Apache-licensed tests in our tree?

Comment 3

[(no longer active)]

(In reply to comment #2)
> Gerv, is it OK to have Apache-licensed tests in our tree?

I'll let Gerv weigh in here, but FWIW I had sent him an email on last March when I was integrating the richtext test suite in our tree (which is released under the same license), and he said that it's OK.

Comment 4

[(no longer active)]

Attachment: Part 2: Add a test driver for the richtext2 suite

This patch adds a test driver to run the richtext2 test suite.  The currentStatus.js file is generated by setting the UPDATE_TEST_RESULTS variable to true.  The exact steps have been documented in README.Mozilla.

Comment 5

[(no longer active)]

Attachment: Part 3: Remove test_set.py

The richtext2 suite includes a file named test_set.py.  Files starting with "test_" have a special meaning to our mochitest runner, and this file confuses it, and mochitest runner tries to run it as a mochitest.

This patch removes files with names beginning with "test_",  This is fine, since this file is not needed to run the test suite.

Comment 6

[Robert O'Callahan (:roc) (email my personal email if necessary)]

The test driver doesn't call todo() at all. How are we going to support tests that we don't pass yet?

The stuff these tests test is not very well specified. How confident are you that the tests are all correct?

Comment 7

[Gervase Markham [:gerv]]

We can't (until we finish, and if we adopt, MPL2) ship Apache code in our binaries, but looks like we have a precedent for taking Apache-licensed tests, so that's fine :-)

Gerv

Comment 8

[(no longer active)]

(In reply to comment #6)
> The test driver doesn't call todo() at all. How are we going to support tests
> that we don't pass yet?

We use the data encoded in currentStatus.js.  The current contents of the file represent our current state in terms of test failures.

I did it this way because this test suite is mostly intended to catch regressions.  Whenever I fix something which makes us pass one of the richtext2 tests that we used to fail on, I'll get a test failure when running this locally, and I will investigate and change currentStatus.js to reflect the new status.

> The stuff these tests test is not very well specified. How confident are you
> that the tests are all correct?

I've been involved in the decision making process for some of the things that this suite tests.  But I haven't read every test in this suite.  But I think that's fine, since I'm not going to spend time to just make us score higher on this test.

The main reason I'm integrating this test into our suite is to make sure that we don't change editor APIs unexpectedly.  This is a much needed coverage which we don't currently have.  If I find a test which is incorrect, I'll fix it in our tree and will upstream the fix to the browserscope project.

Does that make sense?

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

(In reply to comment #8)
> We use the data encoded in currentStatus.js.  The current contents of the file
> represent our current state in terms of test failures.
> 
> I did it this way because this test suite is mostly intended to catch
> regressions.  Whenever I fix something which makes us pass one of the richtext2
> tests that we used to fail on, I'll get a test failure when running this
> locally, and I will investigate and change currentStatus.js to reflect the new
> status.

So currently the tests that are "expected fail", we'll report as "pass". Would it be hard to report them as "todo" instead?

Comment 10

[(no longer active)]

Attachment: Part 2: Add a test driver for the richtext2 suite

(In reply to comment #9)
> (In reply to comment #8)
> > We use the data encoded in currentStatus.js.  The current contents of the file
> > represent our current state in terms of test failures.
> > 
> > I did it this way because this test suite is mostly intended to catch
> > regressions.  Whenever I fix something which makes us pass one of the richtext2
> > tests that we used to fail on, I'll get a test failure when running this
> > locally, and I will investigate and change currentStatus.js to reflect the new
> > status.
> 
> So currently the tests that are "expected fail", we'll report as "pass". Would
> it be hard to report them as "todo" instead?

Yes.  I modified the suite to differentiate between pass, todo, unexpected-pass and regressions.

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 520468 [details] [diff] [review]
Part 2: Add a test driver for the richtext2 suite

+                // Skip the knwon properties

known

Comment 12

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/744bd77a56c6
http://hg.mozilla.org/mozilla-central/rev/ab05f1da44fb
http://hg.mozilla.org/mozilla-central/rev/16ff3ab0d1ff