On Fri, Nov 11, 2011 at 08:18:03PM +0400, Alexey Shumkin wrote: > Firefox on Windows by default is placed in "C:\Program Files\Mozilla Firefox" > folder, i.e. its path contains spaces. Before running this browser "git-web--browse" > tests version of Firefox to decide whether to use "-new-tab" option or not. > > Quote browser path to avoid error during this test. Thanks. I even noticed this bug early on in the previous discussion: http://article.gmane.org/gmane.comp.version-control.git/181600 but forgot about it by the time the final patch rolled around. Your fix looks correct, but: > test_expect_success \ > + 'Firefox below v2.0 paths are properly quoted' ' > + echo fake: http://example.com/foo >expect && > + cat >"fake browser" <<-\EOF && > + #!/bin/sh > + > + if [ "$1" == "-version" ]; then Using "==" is a bashism. Just use "=". Also, a style nit, but we usually spell this "test" and not "[". I admit I don't care much, though. > + # Firefox (in contrast to w3m) is run in background (with &) > + # so redirect output to "actual" > + echo fake: "$@" > actual > + fi > + EOF > + chmod +x "fake browser" && > + git config browser.firefox.path "`pwd`/fake browser" && > + git web--browse --browser=firefox \ > + http://example.com/foo && > + test_cmp expect actual Hmm. So we are running the fake browser in the background, but then check that it has written something as soon as web--browse exits. Isn't that a race condition? I.e., we could run "test_cmp" before the browser has actually written anything? I'm not sure there's a good way to do it. You would need either to wait some pre-determined "it could not possibly take it longer than N seconds to run" sleep, or we need some kind of synchronization point. We can't wait call "wait" on the child PID (if we even have it, because it's not our child). -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html