[Testing] Convert DumpRenderTree to use generated test header options
Created attachment 411678 [details] Patch
Created attachment 411680 [details] Patch
Created attachment 411688 [details] Patch
Going to try making the VPATH include the erb files.
If anyone has any idea what I am doing wrong with these Makefile rules, I could really use a pointer.
Specifically, it's these errors: /bin/sh -c \"/Volumes/Data/worker/tvOS-14-Simulator-Build-EWS/build/WebKitBuild/DumpRenderTree.build/Release-appletvsimulator/Derived\ Sources.build/Script-0F18E7011D6B9CC60027E547.sh\" make: Circular /Volumes/Data/worker/tvOS-14-Simulator-Build-EWS/build/Tools/DumpRenderTree/Scripts/PreferencesTemplates/TestOptionsGeneratedWebKitLegacyKeyMapping.cpp.erb.cpp <- /Volumes/Data/worker/tvOS-14-Simulator-Build-EWS/build/Tools/DumpRenderTree/Scripts/PreferencesTemplates/TestOptionsGeneratedWebKitLegacyKeyMapping.cpp.erb dependency dropped. make: Circular /Volumes/Data/worker/tvOS-14-Simulator-Build-EWS/build/Tools/DumpRenderTree/Scripts/PreferencesTemplates/TestOptionsGeneratedKeys.h.erb.sh <- /Volumes/Data/worker/tvOS-14-Simulator-Build-EWS/build/Tools/DumpRenderTree/Scripts/PreferencesTemplates/TestOptionsGene ratedWebKitLegacyKeyMapping.cpp.erb dependency dropped. make: Circular /Volumes/Data/worker/tvOS-14-Simulator-Build-EWS/build/Tools/DumpRenderTree/Scripts/PreferencesTemplates/TestOptionsGeneratedKeys.h.erb.sh <- /Volumes/Data/worker/tvOS-14-Simulator-Build-EWS/build/Tools/DumpRenderTree/Scripts/PreferencesTemplates/TestOptionsGeneratedKeys.h.erb dependency dropped. ruby /Volumes/Data/worker/tvOS-14-Simulator-Build-EWS/build/WebKitBuild/Release-appletvsimulator/usr/local/include/wtf/Scripts/GeneratePreferences.rb --frontend WebKitLegacy --base /Volumes/Data/worker/tvOS-14-Simulator-Build-EWS/build/WebKitBuild/Release-appletvsimulator/usr/local/include/wtf/Scripts/Preferences/WebPreferences.yaml --debug /Volumes/Data/worker/tvOS-14-Simulator-Build-EWS/build/WebKitBuild/Release-appletvsimulator/usr/local/include/wtf/Scripts/Preferences/WebPreferencesDebug.yaml --experimental /Volumes/Data/worker/tvOS-14-Simulator-Build-EWS/build/WebKitBuild/Release-appletvsimulator/usr/local/include/wtf/Scripts/Preferences/WebPreferencesExperimental.yaml --internal /Volumes/Data/worker/tvOS-14-Simulator-Build-EWS/build/WebKitBuild/Release-appletvsimulator/usr/local/include/wtf/Scripts/Preferences/WebPreferencesInternal.yaml --template /Volumes/Data/worker/tvOS-14-Simulator-Build-EWS/build/Tools/DumpRenderTree/Scripts/PreferencesTemplates/TestOptionsGeneratedWebKitLegacyKeyMapping.cpp.erb --template /Volumes/Data/worker/tvOS-14-Simulator-Build-EWS/build/Tools/DumpRenderTree/Scripts/PreferencesTemplates/TestOptionsGeneratedKeys.h.erb cat /Volumes/Data/worker/tvOS-14-Simulator-Build-EWS/build/Tools/DumpRenderTree/Scripts/PreferencesTemplates/TestOptionsGeneratedKeys.h.erb.sh >/Volumes/Data/worker/tvOS-14-Simulator-Build-EWS/build/Tools/DumpRenderTree/Scripts/PreferencesTemplates/TestOptionsGeneratedKeys.h.erb cat: /Volumes/Data/worker/tvOS-14-Simulator-Build-EWS/build/Tools/DumpRenderTree/Scripts/PreferencesTemplates/TestOptionsGeneratedKeys.h.erb.sh: No such file or directory I don't understand where the .sh suffixes are coming from, but I am clearly doing something quite wrong.
I don’t see a "cat" command anywhere in DerivedSources.make.
Comment on attachment 411688 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=411688&action=review Trying to figure this out but so far nothing. > Tools/DumpRenderTree/DerivedSources.make:32 > +VPATH = \ > + $(UISCRIPTCONTEXT_DIR) \ > + $(PREFERENCES_TEMPLATE_DIR) \ > +# I am not sure what we are trying to accomplish here. VPATH tells make where to *search* for prerequisites. I don’t think we need a VPATH to make a rule work when the prerequisites are full paths, so I don’t see why PREFERENCES_TEMPLATE_DIR needs to be in a VPATH. > Tools/DumpRenderTree/DerivedSources.make:46 > WEB_PREFERENCES_FILES = $(basename $(notdir $(WEB_PREFERENCES_TEMPLATES))) WEB_PREFERENCES_FILES will be: TestOptionsGeneratedWebKitLegacyKeyMapping.cpp TestOptionsGeneratedKeys.h > Tools/DumpRenderTree/DerivedSources.make:47 > +WEB_PREFERENCES_PATTERNS = $(subst .,%,$(WEB_PREFERENCES_FILES)) WEB_PREFERENCES_PATTERNS will be: TestOptionsGeneratedWebKitLegacyKeyMapping%cpp TestOptionsGeneratedKeys%h > Tools/DumpRenderTree/DerivedSources.make:49 > all : $(WEB_PREFERENCES_FILES) A little strange to have this *before* the .PHONY : all rule below.
Created attachment 411689 [details] Patch
Try a more traditional Makefile pattern rule here: %.cpp : %.cpp.erb $(WTF_BUILD_SCRIPTS_DIR)/GeneratePreferences.rb $(WEB_PREFERENCES_TEMPLATES) $(WEB_PREFERENCES) $(RUBY) $(WTF_BUILD_SCRIPTS_DIR)/GeneratePreferences.rb --frontend WebKitLegacy --base ${WTF_BUILD_SCRIPTS_DIR}/Preferences/WebPreferences.yaml --debug ${WTF_BUILD_SCRIPTS_DIR}/Preferences/WebPreferencesDebug.yaml --experimental ${WTF_BUILD_SCRIPTS_DIR}/Preferences/WebPreferencesExperimental.yaml --internal ${WTF_BUILD_SCRIPTS_DIR}/Preferences/WebPreferencesInternal.yaml --template $< %.h : %.h.erb $(WTF_BUILD_SCRIPTS_DIR)/GeneratePreferences.rb $(WEB_PREFERENCES_TEMPLATES) $(WEB_PREFERENCES) $(RUBY) $(WTF_BUILD_SCRIPTS_DIR)/GeneratePreferences.rb --frontend WebKitLegacy --base ${WTF_BUILD_SCRIPTS_DIR}/Preferences/WebPreferences.yaml --debug ${WTF_BUILD_SCRIPTS_DIR}/Preferences/WebPreferencesDebug.yaml --experimental ${WTF_BUILD_SCRIPTS_DIR}/Preferences/WebPreferencesExperimental.yaml --internal ${WTF_BUILD_SCRIPTS_DIR}/Preferences/WebPreferencesInternal.yaml --template $< I'm not sure what the right way to say the output could either end in a .h or a .cpp here
Created attachment 411693 [details] Patch
This one has make being invoked with make -d, see if we can find anything interesting from the debug logging make has.
Created attachment 411695 [details] Patch
Created attachment 411706 [details] Patch
Created attachment 411707 [details] Patch
Created attachment 411709 [details] Patch
Yay! I think the static patterns work! Never used those before, but they seem like a good fit here.
Yes, that does seem like a great solution. Less exotic than the others, too.
(In reply to Darin Adler from comment #18) > Yes, that does seem like a great solution. Less exotic than the others, too. Now just to fix the test failures.
Created attachment 411711 [details] Patch
Created attachment 411712 [details] Patch
Created attachment 411713 [details] Patch
Created attachment 411714 [details] Patch
Created attachment 411715 [details] Patch
Created attachment 411716 [details] Patch
Created attachment 411723 [details] Patch
Created attachment 411724 [details] Patch
Looks like those tests are passing.
(In reply to Darin Adler from comment #28) > Looks like those tests are passing. The Mac-wk1 bots seem to have hit some issues extracting results, and seem to still be failing. I reached out to the bot-watchers to see what is up.
(In reply to Sam Weinig from comment #29) > The Mac-wk1 bots seem to have hit some issues extracting results, and seem > to still be failing. I reached out to the bot-watchers to see what is up. mac-wk1 and mac-debug-wk1 seems to be consistently failing with 30+ failures. Also the layout-test runs for this patch on these queues have 500,000+ log lines which is unusually high (~10x the usual). Please have a look. e.g.: https://ews-build.webkit.org/#/builders/30/builds/20930 https://ews-build.webkit.org/#/builders/30/builds/20900 https://ews-build.webkit.org/#/builders/30/builds/20894 https://ews-build.webkit.org/#/builders/32/builds/20683 https://ews-build.webkit.org/#/builders/32/builds/20675
Created attachment 412186 [details] Patch
Created attachment 412189 [details] Patch
Comment on attachment 412189 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=412189&action=review > Source/WebKitLegacy/mac/WebView/WebPreferences.mm:2992 > @end > > + > +@implementation WebPreferences (WebPrivateTesting) Maybe just one blank line here? > Source/WebKitLegacy/mac/WebView/WebPreferences.mm:3004 > + RetainPtr<CFHTTPCookieStorageRef> cookieStorage = NetworkStorageSessionMap::defaultStorageSession().cookieStorage(); auto > Source/WebKitLegacy/mac/WebView/WebPreferences.mm:3005 > + ASSERT(cookieStorage); // Will fail when NetworkStorageSessionMap::switchToNewTestingSession() was not called beforehand. Maybe RELEASE_ASSERT? Certainly harmless to do that, and can’t say that about all ASSERT. > Source/WebKitLegacy/mac/WebView/WebPreferencesPrivate.h:306 > +@interface WebPreferences (WebPrivateTesting) I suggest either a blank line after @interface or no blank line before @end.
Created attachment 412190 [details] Patch
Created attachment 412191 [details] Patch
hm, something is just destroying the tests here. 09:56:21.563 29570 worker/9 animations/font-variations/font-stretch.html output stderr lines: 09:56:21.563 29570 2020-10-23 09:56:20.919 DumpRenderTree[29573:69131] NetworkStorageDB:_openDBReadConnections: failed to open read connection to DB @ (null)/Cache.db. Error=14. Cause=unable to open database file 09:56:21.563 29570 2020-10-23 09:56:20.919 DumpRenderTree[29573:69131] CacheRead: unable to open cache files in (null) 09:56:21.563 29570 2020-10-23 09:56:21.436 DumpRenderTree[29573:69131] NetworkStorageDB:_openDBReadConnections: failed to open read connection to DB @ (null)/Cache.db. Error=14. Cause=unable to open database file 09:56:21.563 29570 2020-10-23 09:56:21.436 DumpRenderTree[29573:69131] CacheRead: unable to open cache files in (null) 09:56:21.563 29570 2020-10-23 09:56:21.462 DumpRenderTree[29573:69131] NetworkStorageDB:_openDBReadConnections: failed to open read connection to DB @ (null)/Cache.db. Error=14. Cause=unable to open database file 09:56:21.563 29570 2020-10-23 09:56:21.462 DumpRenderTree[29573:69131] CacheRead: unable to open cache files in (null) I don't see this output when running locally, and can't find any references to NetworkStorageDB, so not sure what is going on yet.
Created attachment 412194 [details] Patch
(In reply to Sam Weinig from comment #36) > hm, something is just destroying the tests here. > > 09:56:21.563 29570 worker/9 animations/font-variations/font-stretch.html > output stderr lines: > 09:56:21.563 29570 2020-10-23 09:56:20.919 DumpRenderTree[29573:69131] > NetworkStorageDB:_openDBReadConnections: failed to open read connection to > DB @ (null)/Cache.db. Error=14. Cause=unable to open database file > 09:56:21.563 29570 2020-10-23 09:56:20.919 DumpRenderTree[29573:69131] > CacheRead: unable to open cache files in (null) > 09:56:21.563 29570 2020-10-23 09:56:21.436 DumpRenderTree[29573:69131] > NetworkStorageDB:_openDBReadConnections: failed to open read connection to > DB @ (null)/Cache.db. Error=14. Cause=unable to open database file > 09:56:21.563 29570 2020-10-23 09:56:21.436 DumpRenderTree[29573:69131] > CacheRead: unable to open cache files in (null) > 09:56:21.563 29570 2020-10-23 09:56:21.462 DumpRenderTree[29573:69131] > NetworkStorageDB:_openDBReadConnections: failed to open read connection to > DB @ (null)/Cache.db. Error=14. Cause=unable to open database file > 09:56:21.563 29570 2020-10-23 09:56:21.462 DumpRenderTree[29573:69131] > CacheRead: unable to open cache files in (null) > > I don't see this output when running locally, and can't find any references > to NetworkStorageDB, so not sure what is going on yet. Finally found NetworkStorageDB in CFNetwork. I broke the cache somehow here.
(In reply to Sam Weinig from comment #38) > (In reply to Sam Weinig from comment #36) > > hm, something is just destroying the tests here. > > > > 09:56:21.563 29570 worker/9 animations/font-variations/font-stretch.html > > output stderr lines: > > 09:56:21.563 29570 2020-10-23 09:56:20.919 DumpRenderTree[29573:69131] > > NetworkStorageDB:_openDBReadConnections: failed to open read connection to > > DB @ (null)/Cache.db. Error=14. Cause=unable to open database file > > 09:56:21.563 29570 2020-10-23 09:56:20.919 DumpRenderTree[29573:69131] > > CacheRead: unable to open cache files in (null) > > 09:56:21.563 29570 2020-10-23 09:56:21.436 DumpRenderTree[29573:69131] > > NetworkStorageDB:_openDBReadConnections: failed to open read connection to > > DB @ (null)/Cache.db. Error=14. Cause=unable to open database file > > 09:56:21.563 29570 2020-10-23 09:56:21.436 DumpRenderTree[29573:69131] > > CacheRead: unable to open cache files in (null) > > 09:56:21.563 29570 2020-10-23 09:56:21.462 DumpRenderTree[29573:69131] > > NetworkStorageDB:_openDBReadConnections: failed to open read connection to > > DB @ (null)/Cache.db. Error=14. Cause=unable to open database file > > 09:56:21.563 29570 2020-10-23 09:56:21.462 DumpRenderTree[29573:69131] > > CacheRead: unable to open cache files in (null) > > > > I don't see this output when running locally, and can't find any references > > to NetworkStorageDB, so not sure what is going on yet. > > Finally found NetworkStorageDB in CFNetwork. I broke the cache somehow here. Alex, Antti, Kilzer, Do any of you know what might be causing this? Seems like the Cache.db path is getting set to null. But I can't quite figure out why (and for unrelated reasons, can't easily debug it locally.)
Comment on attachment 412194 [details] Patch WebKitLegacy networking was mostly before my time, but I would look closely at the call to switchToNewTestingSession and the call to _resetToDefaultValuesForTesting looks like a suspicious change in behavior.
Created attachment 412219 [details] Patch
Comment on attachment 412219 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=412219&action=review > Tools/DumpRenderTree/mac/DumpRenderTree.mm:855 > + // [preferences _resetToDefaultValuesForTesting]; This seems to have done it. Please don't commit this, though.
(In reply to Alex Christensen from comment #42) > Comment on attachment 412219 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=412219&action=review > > > Tools/DumpRenderTree/mac/DumpRenderTree.mm:855 > > + // [preferences _resetToDefaultValuesForTesting]; > > This seems to have done it. Please don't commit this, though. Oh, I'm very well aware it's the reset that is breaking things (and won't commit until I figure it out and fix it), I've done a variety of different reset methods and they all cause the breakage. Ordinarily I would just debug it, but can't at the moment, so am doing triage via the bots :(.
Created attachment 412243 [details] Patch
Created attachment 412244 [details] Patch
<rdar://problem/70653177>
Created attachment 412252 [details] Patch
Created attachment 412253 [details] Experiment: Remove setSharedURLCache and add _resetToDefaultValuesForTesting
Created attachment 412254 [details] Experiment: Remove setSharedURLCache and new WebPreferences per test
Created attachment 412256 [details] Patch
Created attachment 412264 [details] Patch
This ended up being done in via other changes in a slightly different way.