1352813 - Reformat testing/web-platform/tests/editing/{data/*.js,include/tests.js} to be more readable

Status: RESOLVED WORKSFORME

(Core :: DOM: Editor, enhancement, P3)

Description

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

"This is really hard to understand, I think that this and editing/include/tests.js should be rewritten with JSON style."
https://reviewboard.mozilla.org/r/125084/#review127888

What format would you like?

Comment 1

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away until 6/30 except 24)]

For example,

["foo[bar]baz",
    [["defaultparagraphseparator",""]],
    "foo[bar]baz",
    [false],
    {"defaultparagraphseparator":[false,false,"div",false,false,"div"]}],

Could be:

[{ initialContent: "foo[bar]baz",
   execCommands: [{ arguments: ["defaultparagraphseparator", ""],
                    expectedReulst: false}],
   expectedContent: "foo[bar]baz",
   queryBeforeExecCommand: [{ command: "defaultparagraphseparator",
                              expectedResult: { queryCommandIndeterm: false,
                                                queryCommandState: false,
                                                queryCommandValue: "div"} }],
   queryAfterExecCommand: [{ command: "defaultparagraphseparator",
                             expectedResult: { queryCommandIndeterm: false,
                                               queryCommandState: false,
                                               queryCommandValue: "div"} }],
 }],

Then, anybody could understand each item meaning even without comment.

Comment 2

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

The data files are already 1.7M and 29,961 lines.  These tests collectively take a long time to run, and IIRC, a large fraction of test runtime is just reading the files off the network and parsing them.  They also can make diff tools run very slowly.  Do you really want to make all of them several times as long?  I think there are <5 people who need to understand the contents of these files, and it's not very complicated, so it doesn't make sense to make everyone's tests run so much more slowly.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away until 6/30 except 24)]

Hmm, is that causes so slower even on modern machines? But indeed, the file becomes big.

The biggest issue of the file is, there is no document about the format.

Even I understand the meaning of each item, [3] (and [1]) and [4] are not easier to read. [3] is the result of [1], but its really separated. [5] is really messy...

Comment 4

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

The way you read it as: start with [1], then execute [2], then you should get [3], with return value [4], and by the way also check [5].  I agree [5] is messy, but it's not hard to read if you remember "indeterm state value" (alphabetical order).

https://github.com/w3c/web-platform-tests/pull/5318

Can this be closed?

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away until 6/30 except 24)]

I think that at least the head of the file should have the explanation. Otherwise, I need to read editing/include/tests.js again and again when I need to review the change of it.

Comment 6

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

The head of which file?  And what does tests.js have to do with it?

Comment 7

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away until 6/30 except 24)]

The head of editing/data/misc.js because when you need maintain the tests, you must touch misc.js in most cases. However, it's hard to review only with misc.js's diff file because it doesn't have any explanation in it.

Comment 8

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

It's not just misc.js, there are 33 sets of tests there.  I don't think it's a good idea to copy-paste the explanation into all 33 of them.

It might be best to continue the discussion on https://github.com/w3c/web-platform-tests/pull/5318 where this is being reviewed.

Comment 9

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Away until 6/30 except 24)]

Okay.

Comment 10

[Aryeh Gregor (:ayg) (no longer with Mozilla)]

To be discussed upstream if there are further concerns.