Bug 70912 – [NRWT] Parsing of test_expectations.txt should be agnostic to newline at end

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Balazs Kelemen]

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.

[Balazs Kelemen]

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?

[Balazs Ankes]

Created attachment 113638 [details]
proposed fix

[Balazs Kelemen]

As discussed with Balazs I renamed the bug title to be consistent with the patch. Balazs, please reupload the patch with a new changelog.

[Balazs Ankes]

Created attachment 113648 [details]
proposed fix

[Adam Barth]

Comment on attachment 113648 [details]
proposed fix

Seems reasonable, but we need a test.

[Balazs Ankes]

Created attachment 113852 [details]
proposed fix

[Ojan Vafai]

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.

[WebKit Review Bot]

Comment on attachment 113852 [details]
proposed fix

Clearing flags on attachment: 113852

Committed r99469: <http://trac.webkit.org/changeset/99469>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

Comment on attachment 113852 [details]
proposed fix

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