Bug 40136 – webkit-patch upload doesn't show diff on cygwin

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tony Gentilcore]

webkit-patch upload doesn't show diff on cygwin

[Tony Gentilcore]

Created attachment 57808 [details]
Patch

[David Levin]

Hmmm... no test. :( (Don't know how possible it is or not.)

[Tony Gentilcore]

(In reply to comment #2)
> Hmmm... no test. :( (Don't know how possible it is or not.)

Manually, I tested this on osx to verify that the pretty diff is still displayed and cygwin to verify that the plain old diff is displayed. However, test-webkitpy hangs for me on cygwin so I haven't verified the full suite passes.

If you think a test is important, I can update the mock to control the return value of can_open_url() and add a test that exercises the new branch in confirmdiff. But I can't really test can_open_url() itself.

By the way, I'm starting to suspect that no one else is developing webkit on windows+cygwin. Should I just leave it be and switch to mac?

[Tony Gentilcore]

By the way, the pretty diff file is successfully created in C:\cygwin\tmp. An alternative approach to adding can_open_url() would be to modify open_url() to test webbrowser.get() internally and on exception just output the file:///{{CYGWIN_PATH}}/tmp/{{TEMP_FILE}} URL to the console so the user can manually open it.

[Adam Barth]

Comment on attachment 57808 [details]
Patch

Looks reasonable.  Generally, we don't have a good strategy for testing things in user.py.  In fact, that file exists to abstract away things that we don't know how to test.

[WebKit Commit Bot]

Comment on attachment 57808 [details]
Patch

Clearing flags on attachment: 57808

Committed r60666: <http://trac.webkit.org/changeset/60666>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.