Bug 80717 – webkit-patch optimize-baselines sometimes creates a mac-future result

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

[Ojan Vafai]

1. Sync to r110230.
2. webkit-patch rebaseline-test "Webkit Mac10.7" "svg/batik/masking/maskRegions.svg"
3. webkit-patch optimize-baselines "svg/batik/masking/maskRegions.svg"

After step 2, LayoutTests/platform/chromium-mac/svg/batik/masking/maskRegions-expected.png is added as a new file. After step 3, git status shows me: 
#	new file:   LayoutTests/platform/chromium-mac/svg/batik/masking/maskRegions-expected.png
#	renamed:    LayoutTests/platform/mac/svg/batik/masking/maskRegions-expected.png -> LayoutTests/platform/mac-future/svg/batik/masking/maskRegions-expected.png

Obviously, you could skip step 2 and manually add the chromium-mac expected png file.

[Ojan Vafai]

Related: bug 73927 and bug 72748.

[Ojan Vafai]

Here's a BaselineOptimizer unittest that hits the error case in comment 1:

def test_no_add_mac_future(self):
    self._assertOptimization({
        'LayoutTests/platform/mac': '29a1715a6470d5dd9486a142f609708de84cdac8',
        'LayoutTests/platform/win': '453e67177a75b2e79905154ece0efba6e5bfb65d',
        'LayoutTests/platform/mac-snowleopard': 'c43eaeb358f49d5e835236ae23b7e49d7f2b089f',
        'LayoutTests/platform/chromium-mac': 'a9ba153c700a94ae1b206d8e4a75a621a89b4554',
    }, {
        'LayoutTests/platform/mac': '29a1715a6470d5dd9486a142f609708de84cdac8',
        'LayoutTests/platform/win': '453e67177a75b2e79905154ece0efba6e5bfb65d',
        'LayoutTests/platform/mac-snowleopard': 'c43eaeb358f49d5e835236ae23b7e49d7f2b089f',
        'LayoutTests/platform/chromium-mac': 'a9ba153c700a94ae1b206d8e4a75a621a89b4554',
    })

[Ojan Vafai]

Created attachment 131457 [details]
Patch

[Ojan Vafai]

I'm not sure this is the best fix, but it does fix at least this one case without breaking existing tests.

[Dirk Pranke]

Comment on attachment 131457 [details]
Patch

I'm inclined to r- this because I don't think we should have any code anywhere that is referencing directories that shouldn't actually exist. I realize you didn't create this problem, but I think this is just masking the fact that there's an underlying bug, not fixing the underlying bug. 

On the other hand, I could be open to landing this as a workaround for now (since you're presumably blocked on this) and filing another bug to reference this chunk of code to fix it properly.

Adam, WDYT?

[Adam Barth]

Comment on attachment 131457 [details]
Patch

This is a fine workaround.  Another thing you can do is to reject the new_results_by_directory in optimize() if it contains any virtual directories.  That essentially means that the optimization algorithm failed, which happens sometimes.

[Ojan Vafai]

(In reply to comment #6)
> (From update of attachment 131457 [details])
> This is a fine workaround.  Another thing you can do is to reject the new_results_by_directory in optimize() if it contains any virtual directories.  That essentially means that the optimization algorithm failed, which happens sometimes.

I tried that first. I think this is better because there are cases where we will successfully optimize.

[Adam Barth]

Oh, I meant we could do both.  This patch is fine as-is though.

[WebKit Review Bot]

Comment on attachment 131457 [details]
Patch

Clearing flags on attachment: 131457

Committed r110533: <http://trac.webkit.org/changeset/110533>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.