All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alexey Shumkin <alex.crezoff@gmail.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org, Chris Packham <judge.packham@gmail.com>
Subject: Re: [PATCH] git-web--browser: avoid errors in terminal when running Firefox on Windows
Date: Fri, 11 Nov 2011 23:48:30 +0400	[thread overview]
Message-ID: <20111111234830.32dccd87@zappedws> (raw)
In-Reply-To: <20111111183555.GC16055@sigill.intra.peff.net>

> >  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 "=".
Thanks (I have no skills enough in this area)
> 
> Also, a style nit, but we usually spell this "test" and not "[". I
> admit I don't care much, though.

Oh, I see

> 
> > +		# 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?
eeehh... you're right...
but even on slow Windows Cygwin it is passed )

> 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).
hmm... we can delete "actual" file and wait its appearance (with
some timeout), no ? but I didn't see in tests anything like this
> -Peff

  reply	other threads:[~2011-11-11 19:49 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-02  0:44 [PATCHv3] git-web--browse: avoid the use of eval Chris Packham
2011-10-03  9:57 ` Jeff King
     [not found]   ` <1321028283-17307-1-git-send-email-Alex.Crezoff@gmail.com>
2011-11-11 18:35     ` [PATCH] git-web--browser: avoid errors in terminal when running Firefox on Windows Jeff King
2011-11-11 19:48       ` Alexey Shumkin [this message]
2011-11-11 20:26         ` Jeff King
2013-01-25 14:44           ` Alexey Shumkin
2013-01-25 19:49             ` Junio C Hamano
2013-01-26  0:40               ` [PATCH v2 0/2] git-web--browser: avoid errors in terminal when running Alexey Shumkin
2013-01-26  0:40                 ` [PATCH v2 1/2] t9901-git-web--browse.sh: Use "write_script" helper Alexey Shumkin
2013-01-26  0:40                 ` [PATCH v2 2/2] git-web--browser: avoid errors in terminal when running Firefox on Windows Alexey Shumkin
2013-01-25 22:06             ` [PATCH] " Jeff King
2013-01-25 22:52               ` Shumkin Alexey

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20111111234830.32dccd87@zappedws \
    --to=alex.crezoff@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=judge.packham@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.