| Summary: | Add Unittest to commits_for_uploads() | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Matt Lewis <jlewis3> | ||||||
| Component: | Tools / Tests | Assignee: | Matt Lewis <jlewis3> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | ews-watchlist, glenn, jbedard, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | WebKit Nightly Build | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Matt Lewis
2020-03-02 16:30:07 PST
Created attachment 392228 [details]
Patch
Uploading initial patch to test on both git and svn style checkouts.
Comment on attachment 392228 [details]
Patch
Tested run-webkit-test with all permutations of checkouts that we expect.
Comment on attachment 392228 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=392228&action=review > Tools/ChangeLog:26 > + (PortTest.make_port): changed from MockSystemHost to MockHost This deserves a 'why'. You mentioned in person the rationale, you should call that out here. > Tools/Scripts/webkitpy/common/checkout/scm/stub_repository.py:43 > self._filesystem = filesystem Can you check if the SCM constructor does this already? Feels like it should. If it doesn't lets leave this code as is. > Tools/Scripts/webkitpy/common/checkout/scm/stub_repository_unittest.py:46 > + def mock_executive_for_stub_repository(): This function is a bit weird unless you intend to expand it later and add mock command results. My general rule is if you find yourself defining more than one of the host member objects, you should return a host object. Here, I think we should either go back to the mock host or just pass a constructed executive inline to each calcite (and import MockExecutive instead of executive_mock) Created attachment 392308 [details]
Patch
This should address the various comments that were given.
Committed r257796: <https://trac.webkit.org/changeset/257796> |