Summary: | [Win] Unable to reliably run tests in parallel | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Brent Fulgham <bfulgham> | ||||||||||
Component: | Tools / Tests | Assignee: | Brent Fulgham <bfulgham> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | aroben, bfulgham, commit-queue, glenn, pvollan, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | PC | ||||||||||||
OS: | All | ||||||||||||
Bug Depends on: | 160597 | ||||||||||||
Bug Blocks: | |||||||||||||
Attachments: |
|
Description
Brent Fulgham
2015-01-26 18:25:09 PST
Could the different test executables be stomping on each other's preferences? A number of tests change particular preferences while they run. (In reply to comment #2) > Could the different test executables be stomping on each other's > preferences? A number of tests change particular preferences while they run. Definitely. However, I tried to account for this in "libraryPathForDumpRenderTree()", which is based on the Mac implementation. It looks for DUMPRENDERTREE_TEMP in the environment, and uses that as the basis for its local preference storage. This is supposed to be unique per running test instance. I guess the first thing to check is that this is unique for each running test shard. If they are not, that might explain the problem. (In reply to comment #3) > (In reply to comment #2) > > Could the different test executables be stomping on each other's > > preferences? A number of tests change particular preferences while they run. > > Definitely. However, I tried to account for this in > "libraryPathForDumpRenderTree()", which is based on the Mac implementation. > It looks for DUMPRENDERTREE_TEMP in the environment, and uses that as the > basis for its local preference storage. This is supposed to be unique per > running test instance. > > I guess the first thing to check is that this is unique for each running > test shard. If they are not, that might explain the problem. So I added a printf to the libraryPathForDumpRenderTree, and confirmed that each shard is getting its own temporary directory, which it uses to seed the following preferences: CFPreferencesSetAppValue(WebDatabaseDirectoryDefaultsKey, WebCore::pathByAppendingComponent(libraryPath, "Databases").createCFString().get(), kCFPreferencesCurrentApplication); CFPreferencesSetAppValue(WebStorageDirectoryDefaultsKey, WebCore::pathByAppendingComponent(libraryPath, "LocalStorage").createCFString().get(), kCFPreferencesCurrentApplication); CFPreferencesSetAppValue(WebKitLocalCacheDefaultsKey, WebCore::pathByAppendingComponent(libraryPath, "LocalCache").createCFString().get(), kCFPreferencesCurrentApplication); The question, is whether CFPreferencesSetAppValue is sticking these values in the exact same place, causing all the different shards to see the same values. (In reply to comment #4) > The question, is whether CFPreferencesSetAppValue is sticking these values > in the exact same place, causing all the different shards to see the same > values. I'm pretty sure it does. You'll probably also need to audit uses of localUserSpecificStorageDirectory() and roamingUserSpecificStorageDirectory(). (In reply to comment #5) > (In reply to comment #4) > > The question, is whether CFPreferencesSetAppValue is sticking these values > > in the exact same place, causing all the different shards to see the same > > values. > > I'm pretty sure it does. > > You'll probably also need to audit uses of > localUserSpecificStorageDirectory() and > roamingUserSpecificStorageDirectory(). After talking with a few people, I think it may NOT be possible to do this on Windows with CoreFoundation. The CFPreference store is going to end up being shared between the different processes. Maybe you could give each process a unique CFBundle name? That should give them separate preferences. Created attachment 285319 [details]
Patch
There might be additional issues we need to fix, but this patch fixes the failures I see when running the dom tests in parallel. Comment on attachment 285319 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285319&action=review Fantastic! r=me > Tools/Scripts/webkitpy/common/system/path.py:58 > + path = re.sub('/cygdrive/c', 'c:', path) I think this would be better if we could capture the drive letter and pass it to the substitution so that "/cygdrive/d' -> 'd:'. This would be fine to do as a follow-up to this change. Comment on attachment 285319 [details] Patch Clearing flags on attachment: 285319 Committed r204123: <http://trac.webkit.org/changeset/204123> All reviewed patches have been landed. Closing bug. Re-opened since this is blocked by bug 160597 Created attachment 285424 [details]
Patch
Comment on attachment 285424 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=285424&action=review Thanks for improving the change! I'm not sure what the cause of the timeout was that you fixed this time around, but these changes look good on their own merits. If the tests pass please land it. > Tools/Scripts/webkitpy/common/system/path.py:67 > + fileSystemRoot = _CygPath.convert_using_singleton(match.group(1)) Is it possible that someone might run tools from one root (e.g., 'C:'), and also run separate tools from another root (e.g., 'D:') during the same tool run? I guess that's pretty unlikely. Created attachment 285564 [details]
Patch
(In reply to comment #15) > Comment on attachment 285424 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=285424&action=review > > Thanks for improving the change! I'm not sure what the cause of the timeout > was that you fixed this time around, but these changes look good on their > own merits. If the tests pass please land it. > > > Tools/Scripts/webkitpy/common/system/path.py:67 > > + fileSystemRoot = _CygPath.convert_using_singleton(match.group(1)) > > Is it possible that someone might run tools from one root (e.g., 'C:'), and > also run separate tools from another root (e.g., 'D:') during the same tool > run? I guess that's pretty unlikely. Thanks for reviewing :) I updated the patch to handle this case. Created attachment 285826 [details]
Patch
Comment on attachment 285826 [details]
Patch
Excellent! r=me.
Comment on attachment 285826 [details] Patch Clearing flags on attachment: 285826 Committed r204376: <http://trac.webkit.org/changeset/204376> All reviewed patches have been landed. Closing bug. |