Bug 137615 – REGRESSION(r174535): Updated expectations for the layout tests have wrong color (or wrong message)

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

[Carlos Alberto Lopez Perez]

r174535 <http://trac.webkit.org/r174535> has changed the background color of the expectations to red. But the message stills keeps beeing "This anchor should have a green background"

This are the changes on the expectations from r174535: http://sprunge.us/JQPG?diff

As you can see, the color of the anchor is changed to red (bgcolor=#FF0000), but the message inside still says "This anchor should have a green background"

One of the two (the color or the message) is wrong, isn't it?

The affected tests are:

Regressions: Unexpected text-only failures (8)
  css3/selectors3/html/css3-modsel-61.html [ Failure ]
  css3/selectors3/html/css3-modsel-83.html [ Failure ]
  css3/selectors3/xhtml/css3-modsel-61.xml [ Failure ]
  css3/selectors3/xhtml/css3-modsel-83.xml [ Failure ]
  css3/selectors3/xml/css3-modsel-61.xml [ Failure ]
  css3/selectors3/xml/css3-modsel-83.xml [ Failure ]
  fast/selectors/061.html [ Failure ]
  fast/selectors/083.html [ Failure ]

[Benjamin Poulain]

It is intended. The commit r174535 intentionally breaks all support of :not(:link) and :not(:visited).

Eventually we should fix the regression, but in the short term, the benefits outweigh the risks.

Help is welcome if you have time :)

[Carlos Alberto Lopez Perez]

(In reply to comment #1)
> It is intended. The commit r174535 intentionally breaks all support of :not(:link) and :not(:visited).
> 
> Eventually we should fix the regression, but in the short term, the benefits outweigh the risks.
> 
> Help is welcome if you have time :)

Shouldn't then be a better idea to mark the test as failing?

Or at least update the text saying something like:

"This anchor should have a red background until bug #XXXX is fixed. Once that is done, this anchor should have a green background."

???

Otherwise is very confusing and misleading.

[Benjamin Poulain]

(In reply to comment #2)
> (In reply to comment #1)
> > It is intended. The commit r174535 intentionally breaks all support of :not(:link) and :not(:visited).
> > 
> > Eventually we should fix the regression, but in the short term, the benefits outweigh the risks.
> > 
> > Help is welcome if you have time :)
> 
> Shouldn't then be a better idea to mark the test as failing?
> 
> Or at least update the text saying something like:
> 
> "This anchor should have a red background until bug #XXXX is fixed. Once that is done, this anchor should have a green background."
> 
> ???
> 
> Otherwise is very confusing and misleading.

Checking in failing test is a common practice. The reason is those test can still catch unintended changes. If we were to mark them as failing, an intended change would be ignored by the bots.

[Carlos Alberto Lopez Perez]

(In reply to comment #3)
> (In reply to comment #2)
> > (In reply to comment #1)
> > > It is intended. The commit r174535 intentionally breaks all support of :not(:link) and :not(:visited).
> > > 
> > > Eventually we should fix the regression, but in the short term, the benefits outweigh the risks.
> > > 
> > > Help is welcome if you have time :)
> > 
> > Shouldn't then be a better idea to mark the test as failing?
> > 
> > Or at least update the text saying something like:
> > 
> > "This anchor should have a red background until bug #XXXX is fixed. Once that is done, this anchor should have a green background."
> > 
> > ???
> > 
> > Otherwise is very confusing and misleading.
> 
> Checking in failing test is a common practice. The reason is those test can still catch unintended changes. If we were to mark them as failing, an intended change would be ignored by the bots.

And what about to updating the text of the anchor to say that it is red now because of a tracked bug but that it should be green after that bug is fixed?

[Benjamin Poulain]

(In reply to comment #4)
> > Checking in failing test is a common practice. The reason is those test can still catch unintended changes. If we were to mark them as failing, an intended change would be ignored by the bots.
> 
> And what about to updating the text of the anchor to say that it is red now because of a tracked bug but that it should be green after that bug is fixed?

The tests  your listed are standards tests. I don't think we should change them.

[Carlos Alberto Lopez Perez]

(In reply to comment #5)
> (In reply to comment #4)
> > > Checking in failing test is a common practice. The reason is those test can still catch unintended changes. If we were to mark them as failing, an intended change would be ignored by the bots.
> > 
> > And what about to updating the text of the anchor to say that it is red now because of a tracked bug but that it should be green after that bug is fixed?
> 
> The tests  your listed are standards tests. I don't think we should change them.

Ok.

I have rebaselined the expectations for the GTK port for this tests on r174685 <http://trac.webkit.org/r174685> as also removed them from the expected failures for GTK.

Closing