[PATCH 1 of 3] test-ssh: add some flush() to make output deterministic
Gregory Szorc
gregory.szorc at gmail.com
Fri May 11 17:08:18 EDT 2018
On Mon, May 7, 2018 at 6:11 AM, Yuya Nishihara <yuya at tcha.org> wrote:
> # HG changeset patch
> # User Yuya Nishihara <yuya at tcha.org>
> # Date 1521963557 -32400
> # Sun Mar 25 16:39:17 2018 +0900
> # Node ID 2be95f3cc4f8a4b59f7c50af093ea033d1d09c5e
> # Parent 04ceb267271a45ae08352d76a9e91f8037ce53e7
> test-ssh: add some flush() to make output deterministic
>
> We shouldn't rely on buffering mode/state of file handles.
>
I'm accepting this and understand why we need a flush() to make behavior
deterministic.
However, I think we should consider adding the flush() to the code that
invokes the hooks - not the hooks themselves. In theory, hook output is
streamed to clients. So if we want to ensure hook output is seen by clients
soon after it is generated, I think we should be flushing output
automatically when invoking hooks. In the context of wire protocol
requests, perhaps we should always be making the ui object unbuffered?
>
> diff --git a/tests/test-ssh-proto-unbundle.t b/tests/test-ssh-proto-
> unbundle.t
> --- a/tests/test-ssh-proto-unbundle.t
> +++ b/tests/test-ssh-proto-unbundle.t
> @@ -161,10 +161,12 @@ Test pushing to a server that has a pret
> > import sys
> > def hook1line(ui, repo, **kwargs):
> > ui.write(b'ui.write 1 line\n')
> + > ui.flush()
> > return 1
> > def hook2lines(ui, repo, **kwargs):
> > ui.write(b'ui.write 2 lines 1\n')
> > ui.write(b'ui.write 2 lines 2\n')
> + > ui.flush()
> > return 1
> > def hook1lineflush(ui, repo, **kwargs):
> > ui.write(b'ui.write 1 line flush\n')
> @@ -181,21 +183,31 @@ Test pushing to a server that has a pret
> > ui.write_err(b'ui.write_err 1\n')
> > ui.write(b'ui.write 2\n')
> > ui.write_err(b'ui.write_err 2\n')
> + > ui.flush()
> > return 1
> > def hookprintstdout(ui, repo, **kwargs):
> > print('printed line')
> + > sys.stdout.flush()
> > return 1
> > def hookprintandwrite(ui, repo, **kwargs):
> > print('print 1')
> + > sys.stdout.flush()
> > ui.write(b'ui.write 1\n')
> + > ui.flush()
> > print('print 2')
> + > sys.stdout.flush()
> > ui.write(b'ui.write 2\n')
> + > ui.flush()
> > return 1
> > def hookprintstderrandstdout(ui, repo, **kwargs):
> > print('stdout 1')
> + > sys.stdout.flush()
> > print('stderr 1', file=sys.stderr)
> + > sys.stderr.flush()
> > print('stdout 2')
> + > sys.stdout.flush()
> > print('stderr 2', file=sys.stderr)
> + > sys.stderr.flush()
> > return 1
> > EOF
>
> @@ -1046,10 +1058,10 @@ Mixed print() and ui.write() are both ca
> e> adding manifests\n
> e> adding file changes\n
> e> added 1 changesets with 1 changes to 1 files\n
> + e> print 1\n
> e> ui.write 1\n
> + e> print 2\n
> e> ui.write 2\n
> - e> print 1\n
> - e> print 2\n
> e> transaction abort!\n
> e> rollback completed\n
> e> abort: pretxnchangegroup.fail hook failed\n
> @@ -1106,10 +1118,10 @@ Mixed print() and ui.write() are both ca
> e> adding manifests\n
> e> adding file changes\n
> e> added 1 changesets with 1 changes to 1 files\n
> + e> print 1\n
> e> ui.write 1\n
> + e> print 2\n
> e> ui.write 2\n
> - e> print 1\n
> - e> print 2\n
> e> transaction abort!\n
> e> rollback completed\n
> e> abort: pretxnchangegroup.fail hook failed\n
> @@ -1180,10 +1192,10 @@ print() to stdout and stderr both get ca
> e> adding manifests\n
> e> adding file changes\n
> e> added 1 changesets with 1 changes to 1 files\n
> + e> stdout 1\n
> e> stderr 1\n
> + e> stdout 2\n
> e> stderr 2\n
> - e> stdout 1\n
> - e> stdout 2\n
> e> transaction abort!\n
> e> rollback completed\n
> e> abort: pretxnchangegroup.fail hook failed\n
> @@ -1240,10 +1252,10 @@ print() to stdout and stderr both get ca
> e> adding manifests\n
> e> adding file changes\n
> e> added 1 changesets with 1 changes to 1 files\n
> + e> stdout 1\n
> e> stderr 1\n
> + e> stdout 2\n
> e> stderr 2\n
> - e> stdout 1\n
> - e> stdout 2\n
> e> transaction abort!\n
> e> rollback completed\n
> e> abort: pretxnchangegroup.fail hook failed\n
> @@ -1731,10 +1743,10 @@ Shell and Python hooks writing to stdout
> e> shell stderr 1\n
> e> shell stdout 2\n
> e> shell stderr 2\n
> + e> stdout 1\n
> e> stderr 1\n
> + e> stdout 2\n
> e> stderr 2\n
> - e> stdout 1\n
> - e> stdout 2\n
> e> transaction abort!\n
> e> rollback completed\n
> e> abort: pretxnchangegroup.b hook failed\n
> @@ -1795,10 +1807,10 @@ Shell and Python hooks writing to stdout
> e> shell stderr 1\n
> e> shell stdout 2\n
> e> shell stderr 2\n
> + e> stdout 1\n
> e> stderr 1\n
> + e> stdout 2\n
> e> stderr 2\n
> - e> stdout 1\n
> - e> stdout 2\n
> e> transaction abort!\n
> e> rollback completed\n
> e> abort: pretxnchangegroup.b hook failed\n
> diff --git a/tests/test-ssh.t b/tests/test-ssh.t
> --- a/tests/test-ssh.t
> +++ b/tests/test-ssh.t
> @@ -272,12 +272,14 @@ a bad, evil hook that prints to stdout
> $ cat <<EOF > $TESTTMP/badhook
> > import sys
> > sys.stdout.write("KABOOM\n")
> + > sys.stdout.flush()
> > EOF
>
> $ cat <<EOF > $TESTTMP/badpyhook.py
> > import sys
> > def hook(ui, repo, hooktype, **kwargs):
> > sys.stdout.write("KABOOM IN PROCESS\n")
> + > sys.stdout.flush()
> > EOF
>
> $ cat <<EOF >> ../remote/.hg/hgrc
> @@ -456,6 +458,7 @@ stderr from remote commands should be pr
> > def wrappedpush(orig, repo, *args, **kwargs):
> > res = orig(repo, *args, **kwargs)
> > repo.ui.write('local stdout\n')
> + > repo.ui.flush()
> > return res
> >
> > def extsetup(ui):
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20180511/d8502733/attachment.html>
More information about the Mercurial-devel
mailing list