Bug 70912

Summary: [NRWT] Parsing of test_expectations.txt should be agnostic to newline at end
Product: WebKit Reporter: Balazs Kelemen <kbalazs>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, bank, dpranke, kkristof, ojan, ossy, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on:    
Bug Blocks: 64491    
Attachments:
Description Flags
proposed fix
none
proposed fix
abarth: review-
proposed fix none

Description Balazs Kelemen 2011-10-26 06:01:26 PDT
I tried this:
diff --git a/LayoutTests/platform/qt/Skipped b/LayoutTests/platform/qt/Skipped
index ecc5470..587a69f 100644
--- a/LayoutTests/platform/qt/Skipped
+++ b/LayoutTests/platform/qt/Skipped
@@ -2440,9 +2440,9 @@ http/tests/inspector/resource-tree/resource-tree-non-unique-url.html
 
 # inspector/cookie-parser.html is a flaky crash
 # https://bugs.webkit.org/show_bug.cgi?id=62662
-inspector/cookie-parser.html
-inspector/cookie-resource-match.html
-inspector/evaluate-in-page.html
+#inspector/cookie-parser.html
+#inspector/cookie-resource-match.html
+#inspector/evaluate-in-page.html
 
 # [Qt] Assertion fail in CSSPrimitiveValue ctor
 # https://bugs.webkit.org/show_bug.cgi?id=69933
diff --git a/LayoutTests/platform/qt/test_expectations.txt b/LayoutTests/platform/qt/test_expectations.txt
index 8dd25f3..6173e9c 100644
--- a/LayoutTests/platform/qt/test_expectations.txt
+++ b/LayoutTests/platform/qt/test_expectations.txt
@@ -23,3 +23,4 @@ BUGWK67007 DEBUG : fast/ruby/after-block-doesnt-crash.html = CRASH
 BUGWK67007 DEBUG : fast/ruby/after-table-doesnt-crash.html = CRASH
 BUGWK67007 DEBUG : fast/ruby/generated-after-counter-doesnt-crash.html = CRASH
 BUGWK67007 DEBUG : fast/ruby/generated-before-and-after-counter-doesnt-crash.html = CRASH
+BUGWK62662 DEBUG : inspector/cookie-parser.html = CRASH

And the results is quite strange to me:
balazs@kbsuse:~/master_clean> Tools/Scripts/run-webkit-tests --results-directory _tst --no-show-results --no-http
Running new-run-webkit-tests with one child process.
For more parallelism, run new-run-webkit-tests directly.
FAILURES FOR <linux, x86, release, cpu>
Line:26 Extraneous ':' BUGWK67007 DEBUG : inspector/cookie-parser.html = CRASHBUG_SKIPPED SKIP : fast/repaint/block-selection-gap-stale-cache.html = FAIL
Traceback (most recent call last):
  File "/home/balazs/master_clean/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 445, in <module>
    sys.exit(main())
  File "/home/balazs/master_clean/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 440, in main
    return run(port, options, args)
  File "/home/balazs/master_clean/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py", line 103, in run
    manager.parse_expectations()
  File "/home/balazs/master_clean/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py", line 386, in parse_expectations
    port.test_expectations_overrides())
  File "/home/balazs/master_clean/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py", line 709, in __init__
    self._report_errors()
  File "/home/balazs/master_clean/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py", line 794, in _report_errors
    raise ParseError(fatal=True, errors=errors)
webkitpy.layout_tests.models.test_expectations.ParseError: Line:26 Extraneous ':' BUGWK67007 DEBUG : inspector/cookie-parser.html = CRASHBUG_SKIPPED SKIP : fast/repaint/block-selection-gap-stale-cache.html = FAIL

Seems like an NRWT bug.
Comment 1 Balazs Kelemen 2011-10-26 08:31:48 PDT
Ok, I did not add a new line, sorry for the noise :)
BTW, don't you think we should check the expectations file for new line at the end and die with a nice error message if it doesn't have it?
Comment 2 Balazs Ankes 2011-11-04 02:14:44 PDT
Created attachment 113638 [details]
proposed fix
Comment 3 Balazs Kelemen 2011-11-04 05:06:38 PDT
As discussed with Balazs I renamed the bug title to be consistent with the patch. Balazs, please reupload the patch with a new changelog.
Comment 4 Balazs Ankes 2011-11-04 05:16:05 PDT
Created attachment 113648 [details]
proposed fix
Comment 5 Adam Barth 2011-11-04 10:01:09 PDT
Comment on attachment 113648 [details]
proposed fix

Seems reasonable, but we need a test.
Comment 6 Balazs Ankes 2011-11-07 05:54:26 PST
Created attachment 113852 [details]
proposed fix
Comment 7 Ojan Vafai 2011-11-07 09:02:55 PST
Comment on attachment 113852 [details]
proposed fix

Would be nicer to make all the test expectations files not require a newline, but this is fine.
Comment 8 WebKit Review Bot 2011-11-07 13:20:28 PST
Comment on attachment 113852 [details]
proposed fix

Clearing flags on attachment: 113852

Committed r99469: <http://trac.webkit.org/changeset/99469>
Comment 9 WebKit Review Bot 2011-11-07 13:20:33 PST
All reviewed patches have been landed.  Closing bug.
Comment 10 Eric Seidel (no email) 2011-11-07 13:21:37 PST
Comment on attachment 113852 [details]
proposed fix

Yeah, making it not required would have been a better fix, but OK.