Closed Bug 794891 Opened 12 years ago Closed 12 years ago

Don't reorder expected failures in parseFailures.py

Categories

(Testing :: Mochitest, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED
mozilla18

People

(Reporter: ayg, Assigned: ayg)

References

Details

Attachments

(2 files)

I don't see any particular reason to alphabetize them, and the old hand-constructed files weren't alphabetized.
Attached patch PatchSplinter Review
Attachment #665369 - Flags: review?(Ms2ger)
The old files I hand-wrote used just ":" as a separator, not ": ".  Arguably this is marginally harder to read, but these are meant for machine processing, and this saves over 10 KB on test_runtest.html alone.  Although that's only 0.5%, to be fair.

I don't feel strongly about this patch -- I really just wrote it so that my patch in bug 787432 wouldn't be gratuitously unreadable, and figured I'd throw it at you while I was at it.  I guess if I were really being consistent, I'd get rid of indentation too, and make the value something shorter than "true", which could save a further five chars per line -- 2.5%, or about 3% total, for test_runtest.html.  Does that sound useful to you, or is it not worth bothering with?
Attachment #665377 - Flags: review?(Ms2ger)
Comment on attachment 665369 [details] [diff] [review]
Patch

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

wfm
Attachment #665369 - Flags: review?(Ms2ger) → review+
(In reply to :Aryeh Gregor from comment #2)
> Created attachment 665377 [details] [diff] [review]
> Also: Don't add unnecessary spaces to expected failure JSON
> 
> The old files I hand-wrote used just ":" as a separator, not ": ".  Arguably
> this is marginally harder to read, but these are meant for machine
> processing, and this saves over 10 KB on test_runtest.html alone.  Although
> that's only 0.5%, to be fair.
> 
> I don't feel strongly about this patch -- I really just wrote it so that my
> patch in bug 787432 wouldn't be gratuitously unreadable, and figured I'd
> throw it at you while I was at it.  I guess if I were really being
> consistent, I'd get rid of indentation too, and make the value something
> shorter than "true", which could save a further five chars per line -- 2.5%,
> or about 3% total, for test_runtest.html.  Does that sound useful to you, or
> is it not worth bothering with?

It doesn't feel particularly useful to me, but I don't care particularly strongly as long as we don't stick the entire file on one line (my editor really doesn't like that).
Sticking the file on one line is dumb because then it's impossible to tell if a diff adds or removes lines.
Attachment #665377 - Flags: review?(Ms2ger)
https://hg.mozilla.org/mozilla-central/rev/ea992245125c
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
I've landed

https://hg.mozilla.org/mozilla-central/rev/12453e5547a8

to avoid unnecessarily large diffs. I'd still like to make them consistent, but this should work for now.
You need to log in before you can comment on or make changes to this bug.