Bug 88591

Summary:	Regression: garden-o-matic rebaseline does the wrong thing for missing expectations
Product:	WebKit	Reporter:	Ojan Vafai <ojan>
Component:	Tools / Tests	Assignee:	Nobody <webkit-unassigned>
Status:	RESOLVED FIXED
Severity:	Normal	CC:	abarth, dpranke, eric, tony
Priority:	P2
Version:	528+ (Nightly build)
Hardware:	Unspecified
OS:	Unspecified

Description Ojan Vafai 2012-06-07 16:32:40 PDT

If a test result is MISSING, rebaseline does nothing. Ideally it should rebaseline only the missing result, but I don't think we currently communicate that to garden-o-matic, so it might need to just rebaseline both the txt and png files.

We should verify that webkit-patch rebaseline-expecations doesn't also have this bug now.

Comment 1 Dirk Pranke 2012-06-07 16:35:12 PDT

hm. I would have thought we should do nothing for MISSING files, but I suppose I can see the argument for just adding them.

We don't know whether the missing result is txt, png, or both, though, so I don't know how we can avoid trying to fetch both.

after the patch I just landed, rebaseline-expectations will also ignore the file. In that case, though, the user could just change 'MISSING' to TEXT/IMAGE/IMAGE+TEXT as appropriate to work around the issue.

Comment 2 Ojan Vafai 2012-06-07 16:40:47 PDT

(In reply to comment #1)
> hm. I would have thought we should do nothing for MISSING files, but I suppose I can see the argument for just adding them.

This happens all the time because someone will only add the png for one platform, e.g. http://trac.webkit.org/changeset/119760.

> We don't know whether the missing result is txt, png, or both, though, so I don't know how we can avoid trying to fetch both.

I think that's fine. We can put in a FIXME to properly communicate with result is missing in the future. In the meantime, there will be the possibility that rebaseline will do the wrong thing in rare cases. Almost always, this happens for new tests, so I don't think getting this right is super urgent.

> after the patch I just landed, rebaseline-expectations will also ignore the file. In that case, though, the user could just change 'MISSING' to TEXT/IMAGE/IMAGE+TEXT as appropriate to work around the issue.

I'm OK with that for this case. Would be good for the tool at least to give some feedback that it's skipping this though instead of silently doing nothing.

Comment 3 Dirk Pranke 2012-06-07 16:44:42 PDT

actually, now that I think about it, I bet rebaseline-expectations will call webkit-patch rebaseline-test --suffixes BUILDER_NAME TEST_NAME , i.e., it'll do the wrong thing.

Comment 4 Ojan Vafai 2012-06-28 10:13:42 PDT

This has been fixed.