| Summary: | [Qt][WK2] Refactor on Qt5 Layout tests' structure | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Jesus Sanchez-Palencia <jesus> | ||||||
| Component: | Tools / Tests | Assignee: | Jesus Sanchez-Palencia <jesus> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | kbalazs, ossy, webkit.review.bot | ||||||
| Priority: | P2 | Keywords: | Qt, QtTriaged | ||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 77380 | ||||||||
| Attachments: |
|
||||||||
|
Description
Jesus Sanchez-Palencia
2012-01-31 10:18:44 PST
Created attachment 124786 [details]
Patch
(In reply to comment #1) > Created an attachment (id=124786) [details] > Patch Ossy, it would be cool if you could review this. I didn't include the whole "git mv qt-wk2 qt-5.0-wk2" on this patch because its size was getting to 6.2 MB. If I get the r+ on this I will land it all together manually. Attachment 124786 [details] did not pass style-queue:
Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9
Updating OpenSource
First, rewinding head to replay your work on top of it...
Applying: Fix compilation errors on build-webkit --debug --no-workers on mac.
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/qt/Skipped
CONFLICT (content): Merge conflict in LayoutTests/platform/qt/Skipped
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Auto-merging Source/WebCore/WebCore.exp.in
Auto-merging Source/WebKit/mac/ChangeLog
CONFLICT (content): Merge conflict in Source/WebKit/mac/ChangeLog
Failed to merge in the changes.
Patch failed at 0001 Fix compilation errors on build-webkit --debug --no-workers on mac.
When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".
rebase refs/remotes/origin/master: command returned error: 1
Died at Tools/Scripts/update-webkit line 164.
If any of these errors are false positives, please file a bug against check-webkit-style.
Now we can skip tests on different ways: - qt-4.8: Qt4.8-WK1 - qt-5.0: Qt5.0-WK1 and Qt5.0-WK2 - qt-wk1: Qt4.8-WK1 and Qt5.0-WK1 - qt-wk2: Qt5.0-WK2 Which is missing that we can't skip a test for only Qt5.0-WK1. With your change we can skip tests on different ways: - qt-4.8: Qt4.8-WK1 - qt-5.0: Qt5.0-WK1 and Qt5.0-WK2 - qt-5.0-wk1: Qt5.0-WK1 - qt-5.0-wk2: Qt5.0-WK2 But now we can't skip tests for Qt4.8-WK1 and Qt5.0-WK1 (recent qt-wk1), and you added duplicated entries to qt-4.8 and qt-5.0-wk1 skipped lists. What if we add qt-5.0-wk1 and qt-5.0-wk2 and don't remove the general qt-wk1? Otherwise I think it would be better if we - remove obsolete (4 month old) png files (introduced in http://trac.webkit.org/changeset/95412) from qt-wk2 before this patch lands and reland updated png files after https://bugs.webkit.org/show_bug.cgi?id=70484 fixed. - land the future r+ -ed patch from svn to preserv the Skipped list histories. (use svn cp, move, etc.) (In reply to comment #4) > But now we can't skip tests for Qt4.8-WK1 and Qt5.0-WK1 (recent qt-wk1), > and you added duplicated entries to qt-4.8 and qt-5.0-wk1 skipped lists. > What if we add qt-5.0-wk1 and qt-5.0-wk2 and don't remove the general qt-wk1? I know it might sound weird but from my brief experience with gardening I'd rather have to replicate this tests in qt-4.8 and qt-5.0-wk1. It seemed to me more natural just to skip or unskip tests directly from a list that actually tells what it is dealing with... (In reply to comment #5) > - remove obsolete (4 month old) png files (introduced in http://trac.webkit.org/changeset/95412) from qt-wk2 before this patch lands and reland updated png files after https://bugs.webkit.org/show_bug.cgi?id=70484 fixed. > - land the future r+ -ed patch from svn to preserv the Skipped list histories. > (use svn cp, move, etc.) Not sure I'm following you here... Why do need to remove the png files and add new ones? Should I do that or one of you guys will deal with it when https://bugs.webkit.org/show_bug.cgi?id=70484 is fixed ? Thanks for the review! (In reply to comment #6) > (In reply to comment #4) > > But now we can't skip tests for Qt4.8-WK1 and Qt5.0-WK1 (recent qt-wk1), > > and you added duplicated entries to qt-4.8 and qt-5.0-wk1 skipped lists. > > What if we add qt-5.0-wk1 and qt-5.0-wk2 and don't remove the general qt-wk1? > > I know it might sound weird but from my brief experience with gardening I'd rather have to replicate this tests in qt-4.8 and qt-5.0-wk1. It seemed to me more natural just to skip or unskip tests directly from a list that actually tells what it is dealing with... It's all the same for me, I can agree with you if there are only a couple of duplicated tests. > (In reply to comment #5) > > - remove obsolete (4 month old) png files (introduced in http://trac.webkit.org/changeset/95412) from qt-wk2 before this patch lands and reland updated png files after https://bugs.webkit.org/show_bug.cgi?id=70484 fixed. > > - land the future r+ -ed patch from svn to preserv the Skipped list histories. > > (use svn cp, move, etc.) > > Not sure I'm following you here... Why do need to remove the png files and add new ones? Should I do that or one of you guys will deal with it when https://bugs.webkit.org/show_bug.cgi?id=70484 is fixed ? Because these png files are absolutely obsolete ... And I don't know when will https://bugs.webkit.org/show_bug.cgi?id=70484 be fixed and when can we enable pixel tests. But I don't think if in the near future. So I prefer removing obsolete png files instead of moving them with a huge patch. Balázs, have you got any objection against removing these obsolete png's? (In reply to comment #8) > Balázs, have you got any objection against removing these obsolete png's? Please, do that! :) (In reply to comment #9) > (In reply to comment #8) > > Balázs, have you got any objection against removing these obsolete png's? > > Please, do that! :) Done. ;) Created attachment 126312 [details]
updated patch with svn cp/mv to keep the history
Comment on attachment 126312 [details]
updated patch with svn cp/mv to keep the history
r=me and I'll land the patch manually from svn.
(In reply to comment #12) > (From update of attachment 126312 [details]) > r=me and I'll land the patch manually from svn. Thank you, Ossy! I'm rushing a few things here but please let me know if there is anything else I need to do after this, ok? Thanks again! Comment on attachment 126312 [details] updated patch with svn cp/mv to keep the history Landed in http://trac.webkit.org/changeset/107397 and http://trac.webkit.org/changeset/107398 |