1331846 - Error when running layout/reftests/w3c-css/submitted/check-for-references.sh

Status: RESOLVED FIXED

(Core :: Layout, defect)

Description

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

$ layout/reftests/w3c-css/submitted/check-for-references.sh 
Unexpected type skip-if(winWidget&&isDebugBuild) for ./include
grep: ./include: No such file or directory
Missing link for ./include
Unexpected type skip-if(winWidget&&isDebugBuild) for ./include
grep: ./include: No such file or directory
Missing link for ./include

That's because the "skip-if(winWidget&&isDebugBuild)" added before the "include", which causes the script fails.
http://searchfox.org/mozilla-central/rev/790b2cb423ea1ecb5746722d51633caec9bab95a/layout/reftests/w3c-css/submitted/reftest.list#58,66

Comment 1

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Attachment: Bug 1331846 - Exclude the lines which have "include" appeared in the middle.

Remove '^' from the first "grev -v" since there might be "skip-if"" or other
<failure-type> conditions before the "include".

DONTBUILD because this patch modifies a script checking for the validity of
reftests, whics is NPOTB.

Review commit: https://reviewboard.mozilla.org/r/105350/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/105350/

Comment 2

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

David, I need your help to review check-for-references.sh because you're the author.

Comment 3

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 8827740 [details]
Bug 1331846 - Exclude the lines which have "include" appeared in the middle.

https://reviewboard.mozilla.org/r/105350/#review106506

::: layout/reftests/w3c-css/submitted/check-for-references.sh:6
(Diff revision 1)
>  #!/bin/bash
>  
>  cd "$(dirname "$0")"
>  find . -name reftest.list | sed 's,/reftest.list$,,' | while read DIRNAME
>  do
> -    cat "$DIRNAME/reftest.list" | grep -v "^\(include\|default-preferences\)" | sed 's/ #.*//;s/^#.*//;s/.* == /== /;s/.* != /!= /' | grep -v "^ *$" | while read TYPE TEST REF
> +    cat "$DIRNAME/reftest.list" | grep -v "\(include\|default-preferences\)" | sed 's/ #.*//;s/^#.*//;s/.* == /== /;s/.* != /!= /' | grep -v "^ *$" | while read TYPE TEST REF

This seems a little too broad -- removing anything with "include" might remove some tests.

I'd prefer if you:
 - require a space after
 - require either a space or start-of-line before

Though if the latter is hard, I'd probably be OK with just the former.

Comment 4

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8827740 [details]
Bug 1331846 - Exclude the lines which have "include" appeared in the middle.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/105350/diff/1-2/

Comment 5

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8827740 [details]
Bug 1331846 - Exclude the lines which have "include" appeared in the middle.

https://reviewboard.mozilla.org/r/105350/#review106506

> This seems a little too broad -- removing anything with "include" might remove some tests.
> 
> I'd prefer if you:
>  - require a space after
>  - require either a space or start-of-line before
> 
> Though if the latter is hard, I'd probably be OK with just the former.

I'll go with the former "require a space after". If the test is `== include.html include-ref.html`, it will be removed by the latter method because there's a space before the "include.html"

In patchset 2, I split the "include" and "default-preference" matching rules because a standalone "default-preferences" [1] is allowed.

[1] https://dxr.mozilla.org/mozilla-central/rev/b3774461acc6bee2216c5f57e167f9e5795fb09d/layout/reftests/w3c-css/submitted/masking/reftest.list#108

Comment 6

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Comment on attachment 8827740 [details]
Bug 1331846 - Exclude the lines which have "include" appeared in the middle.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/105350/diff/2-3/

Comment 7

[Ting-Yu Lin [:TYLin] (PST, UTC-8)]

Addressed the comments in comment 3. Please take a look again.

Comment 8

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 8827740 [details]
Bug 1331846 - Exclude the lines which have "include" appeared in the middle.

https://reviewboard.mozilla.org/r/105350/#review106734

Ship It!

Comment 9

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

Comment on attachment 8827740 [details]
Bug 1331846 - Exclude the lines which have "include" appeared in the middle.

https://reviewboard.mozilla.org/r/105350/#review106738

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fe22af79bacf
Exclude the lines which have "include" appeared in the middle. r=dbaron

Comment 11

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/fe22af79bacf