New issue
Advanced search Search tips

Issue 647395 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 2016
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug

Blocking:
issue 621599



Sign in to add a comment

w3c-test-autoroller adds extra newline after marker comment.

Project Member Reported by qyears...@chromium.org, Sep 15 2016

Issue description

When w3c-test-autoroller (https://build.chromium.org/p/chromium.infra.cron/builders/w3c-test-autoroller) has no lines to add to TestExpectations but it finds a marker comment in that file, it unnecessarily adds a blank line below the marker comment.

Example CL: https://codereview.chromium.org/2331373005

When investigating this, I realized that update_w3c_test_expectations.py that doesn't re-use the test expectations parsing logic that's usually used to parse the test expectations file, but I think it should.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 15 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/5ad02862242995ad53cf3e473aed5aa7b919153e

commit 5ad02862242995ad53cf3e473aed5aa7b919153e
Author: qyearsley <qyearsley@chromium.org>
Date: Thu Sep 15 22:22:10 2016

Move tokenize_line method from TestExpectationParser to TestExpectationLine.

In this CL:
 - Move tokenize_line class method to TestExpectationLine, as suggested by FIXME.
 - Make tokenize_line non-protected, since it's used in another class, and
   would be useful in other modules, e.g. update_w3c_test_expectations.
 - Move class attributes that tokenize_line depends on.
 - Other changes suggested by linter (e.g. shorten long variable name).

This is a preliminary refactoring CL which is intended to make it easier to re-use tokenize_line in update_w3c_test_expectations.py, in order to make it easier to fix  http://crbug.com/647395 .

BUG= 647395 

Review-Url: https://codereview.chromium.org/2341173002
Cr-Commit-Position: refs/heads/master@{#419013}

[modify] https://crrev.com/5ad02862242995ad53cf3e473aed5aa7b919153e/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/models/test_expectations.py
[modify] https://crrev.com/5ad02862242995ad53cf3e473aed5aa7b919153e/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/models/test_expectations_unittest.py
[modify] https://crrev.com/5ad02862242995ad53cf3e473aed5aa7b919153e/third_party/WebKit/Tools/Scripts/webkitpy/layout_tests/views/buildbot_results.py

Project Member

Comment 2 by bugdroid1@chromium.org, Sep 16 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d8df377160ed2d41799df0ddfa30f44065059ee7

commit d8df377160ed2d41799df0ddfa30f44065059ee7
Author: qyearsley <qyearsley@chromium.org>
Date: Fri Sep 16 18:11:07 2016

In update_w3c_test_expectations use ExpectationLine.tokenize_line and refactor.

In this CL:
 - Make test methods in update_w3c_test_expectations have more realistic examples.
 - Change some lines in write_to_test_expectations to make that method shorter.

This is a refactoring CL (no behavior changes).

BUG= 647395 

Review-Url: https://codereview.chromium.org/2349543002
Cr-Commit-Position: refs/heads/master@{#419215}

[modify] https://crrev.com/d8df377160ed2d41799df0ddfa30f44065059ee7/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations.py
[modify] https://crrev.com/d8df377160ed2d41799df0ddfa30f44065059ee7/third_party/WebKit/Tools/Scripts/webkitpy/w3c/update_w3c_test_expectations_unittest.py

Blocking: 621599
Status: Fixed (was: Assigned)

Sign in to add a comment