Bug 180389

Summary: [build.webkit.org] Simplify imports in mastercfg_unittest.py
Product: WebKit Reporter: Aakash Jain <aakash_jain>
Component: Tools / TestsAssignee: Aakash Jain <aakash_jain>
Status: ASSIGNED    
Severity: Normal CC: aakash_jain, ap, clopez, dbates, dean_johnson, lforschler, ossy
Priority: P2    
Version: Other   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=180390
Attachments:
Description Flags
Proposed patch clopez: review-

Aakash Jain
Reported 2017-12-04 17:50:03 PST
mastercfg_unittest.py has a BuildBotConfigLoader class which is used to import master.cfg. We should use python imports directly. All the code in this class is no longer required. e.g.: _add_webkitpy_to_sys_path method is unnecessary as Buildbot doesn't depend on webkitpy at all. We should remove this class.
Attachments
Proposed patch (5.72 KB, patch)
2017-12-04 17:57 PST, Aakash Jain 		clopez: review-
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Aakash Jain
Comment 1 2017-12-04 17:57:34 PST
Created attachment 328419 [details] Proposed patch
Csaba Osztrogonác
Comment 2 2017-12-05 09:39:35 PST
Comment on attachment 328419 [details] Proposed patch View in context: https://bugs.webkit.org/attachment.cgi?id=328419&action=review > Tools/BuildSlaveSupport/build.webkit.org-config/mastercfg_unittest.py:-32 > - def _add_dependant_modules_to_sys_modules(self): > - from webkitpy.thirdparty.autoinstalled import buildbot > - sys.modules['buildbot'] = buildbot This code was to be able to run mastercfg_unittest.py without installing buildbot and its dependencies. webkitpy autoinstaller installs exactly the same buildbot and twisted version as the build.webkit.org uses.
Aakash Jain
Comment 3 2017-12-05 16:05:08 PST
Good point. I think I need to look into webkitpy more and understand why does webkitpy has buildbot code. Maybe we can move that autoinstaller code here.
Carlos Alberto Lopez Perez
Comment 4 2018-04-17 17:05:46 PDT
Comment on attachment 328419 [details] Proposed patch I'm setting r- because this patch not longer applies on trunk. Also it looks a similar thing has been done on bug 180390. Should we just close this?
Note You need to log in before you can comment on or make changes to this bug.