Bug 31904 – [bzt] Kill WebKitLandingScripts

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Adam Barth]

It is an evil static of doom that prevents us from testing download commands.

[Adam Barth]

Created attachment 43901 [details]
step 1

[Adam Barth]

Created attachment 43902 [details]
step 2

[Adam Barth]

Created attachment 43903 [details]
step 3

[Adam Barth]

Created attachment 43904 [details]
Step 4

[Adam Barth]

Created attachment 43905 [details]
Step 5

[Adam Barth]

Created attachment 43906 [details]
Step 6

[Eric Seidel (no email)]

Comment on attachment 43901 [details]
step 1

Why pass tool?
 119     def setup_for_patch_apply(tool, options):

Seems scm is cleaner there.

This looks good though.

[Eric Seidel (no email)]

Comment on attachment 43902 [details]
step 2

Calling error() is evil and wrong.  We need to fix that at some point.

[Eric Seidel (no email)]

Comment on attachment 43903 [details]
step 3

LGTM.

[Eric Seidel (no email)]

Comment on attachment 43904 [details]
Step 4

Commands already have a self.tool now.  No need to pass tool:
         self._create_changelogs_for_revert(tool.scm(), revision)
 398         self._create_changelogs_for_revert(tool, revision)

If you're going to move away from scm(), we should move to self.tool

[Eric Seidel (no email)]

Comment on attachment 43905 [details]
Step 5

LGTM.  Eventually we need to unify SCM.run_command with this too.

[Eric Seidel (no email)]

Comment on attachment 43906 [details]
Step 6

I intentionally had this take an scm param:
                 commit_message = commit_message_for_this_commit(scm)
 135                 commit_message = scm.commit_message_for_this_commit()

It doesn't belong on SCM.  Our commit message junk doesn't belong in scm.py.

 38     # FIXME: The options should really live on each "Step" object.

I LOVE the idea of a Step object, and even more that options would be automatically assembled from the set of step objects!

rollout used to work just fine:
              comment_text = WebKitLandingScripts.build_and_commit(tool.scm(), options)
 407             # FIXME: This function does not exist!!
 408             # comment_text = WebKitLandingScripts.build_and_commit(tool.scm(), options)
 409             raise ScriptError("OOPS! This option is not implemented (yet).")

Not sure when it broke.  

406410             self._reopen_bug_after_rollout(tool, bug_id, comment_text)

is broken in the current bugzilla however, because there is no longer a "REOPENED" state in the popup.

I guess we could add a big fixme next to commit_message_for_this_commit.  It doesn't belong in scm.py. :(

I'd like you to at least add a FIXME.

[Adam Barth]

Comment on attachment 43901 [details]
step 1

Clearing flags on attachment: 43901

Committed r51431: <http://trac.webkit.org/changeset/51431>

[Adam Barth]

Comment on attachment 43902 [details]
step 2

Clearing flags on attachment: 43902

Committed r51432: <http://trac.webkit.org/changeset/51432>

[Adam Barth]

Comment on attachment 43903 [details]
step 3

Clearing flags on attachment: 43903

Committed r51433: <http://trac.webkit.org/changeset/51433>

[Adam Barth]

Comment on attachment 43904 [details]
Step 4

Clearing flags on attachment: 43904

Committed r51434: <http://trac.webkit.org/changeset/51434>

[Adam Barth]

Comment on attachment 43905 [details]
Step 5

Clearing flags on attachment: 43905

Committed r51435: <http://trac.webkit.org/changeset/51435>

[Adam Barth]

Committed r51437: <http://trac.webkit.org/changeset/51437>