Bug 70912 - [NRWT] Parsing of test_expectations.txt should be agnostic to newline at end
: [NRWT] Parsing of test_expectations.txt should be agnostic to newline at end
Status: RESOLVED FIXED
: WebKit
Tools / Tests
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
:
:
:
: 64491
  Show dependency treegraph
 
Reported: 2011-10-26 06:01 PST by
Modified: 2011-11-07 13:21 PST (History)


Attachments
proposed fix (1.34 KB, patch)
2011-11-04 02:14 PST, Balazs Ankes
no flags Review Patch | Details | Formatted Diff | Diff
proposed fix (1.33 KB, patch)
2011-11-04 05:16 PST, Balazs Ankes
abarth: review-
Review Patch | Details | Formatted Diff | Diff
proposed fix (2.81 KB, patch)
2011-11-07 05:54 PST, Balazs Ankes
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-10-26 06:01:26 PST
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 From 2011-10-26 08:31:48 PST -------
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 From 2011-11-04 02:14:44 PST -------
Created an attachment (id=113638) [details]
proposed fix
------- Comment #3 From 2011-11-04 05:06:38 PST -------
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 From 2011-11-04 05:16:05 PST -------
Created an attachment (id=113648) [details]
proposed fix
------- Comment #5 From 2011-11-04 10:01:09 PST -------
(From update of attachment 113648 [details])
Seems reasonable, but we need a test.
------- Comment #6 From 2011-11-07 05:54:26 PST -------
Created an attachment (id=113852) [details]
proposed fix
------- Comment #7 From 2011-11-07 09:02:55 PST -------
(From update of attachment 113852 [details])
Would be nicer to make all the test expectations files not require a newline, but this is fine.
------- Comment #8 From 2011-11-07 13:20:28 PST -------
(From update of attachment 113852 [details])
Clearing flags on attachment: 113852

Committed r99469: <http://trac.webkit.org/changeset/99469>
------- Comment #9 From 2011-11-07 13:20:33 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #10 From 2011-11-07 13:21:37 PST -------
(From update of attachment 113852 [details])
Yeah, making it not required would have been a better fix, but OK.