You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2010/09/28 19:30:59 UTC
Re: svn commit: r1002313 -
/subversion/trunk/subversion/tests/cmdline/prop_tests.py
pburba@apache.org wrote on Tue, Sep 28, 2010 at 18:38:19 -0000:
> + # Run propget -vR svn:mergeinfo and collect the stdout.
> + exit_code, pg_stdout, pg_stderr = svntest.actions.run_and_verify_svn(
> + None, None, [], 'propget', SVN_PROP_MERGEINFO, '-vR', wc_dir)
> +
exit_code and pg_stderr aren't checked anywhere.
> + # Run propget -vR svn:mergeinfo, redirecting the stdout to a file.
> + arglist = ['svn.exe', 'propget', SVN_PROP_MERGEINFO, '-vR', wc_dir]
s/.exe// ?
> + redir_file = open(redirect_file, 'wb')
> + pg_proc = subprocess.Popen(arglist, stdout=redir_file)
Shouldn't this use the svntest/ infrastructure? Compare
svntest.actions.check_prop().
> + pg_proc.wait()
> + redir_file.close()
> + pg_stdout_redir = open(redirect_file, 'r').readlines()
> +
> + # Check if the redirected output of svn pg -vR is what we expect.
> + #
> + # Currently this fails because the mergeinfo for the three paths is
> + # interleaved and the lines endings are (at least on Windows) a mix
> + # of <CR><LF> and <LF>. See
> + # http://subversion.tigris.org/issues/show_bug.cgi?id=3721#desc1
> + unordered_expected_output = svntest.verify.UnorderedOutput([
> + "Properties on '" + B_path + "':\n",
> + "Properties on '" + C_path + "':\n",
> + "Properties on '" + D_path + "':\n",
> + " svn:mergeinfo\n",
> + " /subversion/branches/1.5.x:872364-874936\n",
> + " /subversion/branches/1.5.x-34184:874657-874741\n",
> + " /subversion/branches/1.5.x-34432:874744-874798\n",
So, 'unordered' also ignores repetitions? (since the last 4 lines
appear only once each, rather than three times each)
Re: svn commit: r1002313 -
/subversion/trunk/subversion/tests/cmdline/prop_tests.py
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Paul Burba wrote on Tue, Sep 28, 2010 at 18:06:36 -0400:
> On Tue, Sep 28, 2010 at 3:30 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> > pburba@apache.org wrote on Tue, Sep 28, 2010 at 18:38:19 -0000:
> >> + # Run propget -vR svn:mergeinfo and collect the stdout.
> >> + exit_code, pg_stdout, pg_stderr = svntest.actions.run_and_verify_svn(
> >> + None, None, [], 'propget', SVN_PROP_MERGEINFO, '-vR', wc_dir)
> >> +
> >
> > exit_code and pg_stderr aren't checked anywhere.
>
> Neither is pg_stdout...but that entire statement was cruft from an
> earlier version of the test; removed it.
>
:-)
> >> + redir_file = open(redirect_file, 'wb')
> >> + pg_proc = subprocess.Popen(arglist, stdout=redir_file)
> >
> > Shouldn't this use the svntest/ infrastructure? Compare
> > svntest.actions.check_prop().
>
> I didn't use it for two reasons:
I didn't actually mean check_prop() specifically, but rather the
infrastructure it uses --- main.run_command() and main.svn_binary.
> First, svntest.actions.check_prop() only supports finding the props on
> a single path (and as far as I can tell that works fine, no issue
> #3721).
>
> Second, Issue #3721 only occurs when the output of 'svn pg -vR' is
> *redirected to a file* - see
> http://subversion.tigris.org/issues/show_bug.cgi?id=3721#desc1.
> check_prop() is (obviously) all processes and pipes underneath the
> covers, so while it may also be possible to show the bug using it, I
> wrote the test to hew as closely to the actual bug I witnessed as
> possible.
>
Aha, so you're using Popen() directly because you need to have svn's
stdout be a file and not a pipe. I'm a bit surprised that we have a bug
that's that sensitive to trigger, but okay. :-)
> >> + pg_proc.wait()
> >> + redir_file.close()
> >> + pg_stdout_redir = open(redirect_file, 'r').readlines()
> >> +
> >> + # Check if the redirected output of svn pg -vR is what we expect.
> >> + #
> >> + # Currently this fails because the mergeinfo for the three paths is
> >> + # interleaved and the lines endings are (at least on Windows) a mix
> >> + # of <CR><LF> and <LF>. See
> >> + # http://subversion.tigris.org/issues/show_bug.cgi?id=3721#desc1
> >> + unordered_expected_output = svntest.verify.UnorderedOutput([
> >> + "Properties on '" + B_path + "':\n",
> >> + "Properties on '" + C_path + "':\n",
> >> + "Properties on '" + D_path + "':\n",
> >> + " svn:mergeinfo\n",
> >> + " /subversion/branches/1.5.x:872364-874936\n",
> >> + " /subversion/branches/1.5.x-34184:874657-874741\n",
> >> + " /subversion/branches/1.5.x-34432:874744-874798\n",
> >
> > So, 'unordered' also ignores repetitions? (since the last 4 lines
> > appear only once each, rather than three times each)
>
> I think you mean the first 3 lines appear only once, all the other
> lines appear 3 times each (because the test sets the same mergeinfo on
> all three paths and the expected output is for svn pg -v***R***).
>
> But yeah, this test isn't perfect as it would allow repetitions. That
> is the price we pay when using svntest.verify.UnorderedOutput(), which
> is required here because there is no guarantee as to the order in
> which svn pg -vR will report the three paths. I tweaked the test to
> check the expected number of lines in the actual redirected output, so
> that we catch any dups.
>
In short, "Yes" (UnorderedOutput() does ignore repetitions), but thanks
for the detailed-as-usual replies!
> Paul
Re: svn commit: r1002313 -
/subversion/trunk/subversion/tests/cmdline/prop_tests.py
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Paul Burba wrote on Tue, Sep 28, 2010 at 18:06:36 -0400:
> On Tue, Sep 28, 2010 at 3:30 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> > pburba@apache.org wrote on Tue, Sep 28, 2010 at 18:38:19 -0000:
> >> + # Run propget -vR svn:mergeinfo and collect the stdout.
> >> + exit_code, pg_stdout, pg_stderr = svntest.actions.run_and_verify_svn(
> >> + None, None, [], 'propget', SVN_PROP_MERGEINFO, '-vR', wc_dir)
> >> +
> >
> > exit_code and pg_stderr aren't checked anywhere.
>
> Neither is pg_stdout...but that entire statement was cruft from an
> earlier version of the test; removed it.
>
:-)
> >> + redir_file = open(redirect_file, 'wb')
> >> + pg_proc = subprocess.Popen(arglist, stdout=redir_file)
> >
> > Shouldn't this use the svntest/ infrastructure? Compare
> > svntest.actions.check_prop().
>
> I didn't use it for two reasons:
I didn't actually mean check_prop() specifically, but rather the
infrastructure it uses --- main.run_command() and main.svn_binary.
> First, svntest.actions.check_prop() only supports finding the props on
> a single path (and as far as I can tell that works fine, no issue
> #3721).
>
> Second, Issue #3721 only occurs when the output of 'svn pg -vR' is
> *redirected to a file* - see
> http://subversion.tigris.org/issues/show_bug.cgi?id=3721#desc1.
> check_prop() is (obviously) all processes and pipes underneath the
> covers, so while it may also be possible to show the bug using it, I
> wrote the test to hew as closely to the actual bug I witnessed as
> possible.
>
Aha, so you're using Popen() directly because you need to have svn's
stdout be a file and not a pipe. I'm a bit surprised that we have a bug
that's that sensitive to trigger, but okay. :-)
> >> + pg_proc.wait()
> >> + redir_file.close()
> >> + pg_stdout_redir = open(redirect_file, 'r').readlines()
> >> +
> >> + # Check if the redirected output of svn pg -vR is what we expect.
> >> + #
> >> + # Currently this fails because the mergeinfo for the three paths is
> >> + # interleaved and the lines endings are (at least on Windows) a mix
> >> + # of <CR><LF> and <LF>. See
> >> + # http://subversion.tigris.org/issues/show_bug.cgi?id=3721#desc1
> >> + unordered_expected_output = svntest.verify.UnorderedOutput([
> >> + "Properties on '" + B_path + "':\n",
> >> + "Properties on '" + C_path + "':\n",
> >> + "Properties on '" + D_path + "':\n",
> >> + " svn:mergeinfo\n",
> >> + " /subversion/branches/1.5.x:872364-874936\n",
> >> + " /subversion/branches/1.5.x-34184:874657-874741\n",
> >> + " /subversion/branches/1.5.x-34432:874744-874798\n",
> >
> > So, 'unordered' also ignores repetitions? (since the last 4 lines
> > appear only once each, rather than three times each)
>
> I think you mean the first 3 lines appear only once, all the other
> lines appear 3 times each (because the test sets the same mergeinfo on
> all three paths and the expected output is for svn pg -v***R***).
>
> But yeah, this test isn't perfect as it would allow repetitions. That
> is the price we pay when using svntest.verify.UnorderedOutput(), which
> is required here because there is no guarantee as to the order in
> which svn pg -vR will report the three paths. I tweaked the test to
> check the expected number of lines in the actual redirected output, so
> that we catch any dups.
>
In short, "Yes" (UnorderedOutput() does ignore repetitions), but thanks
for the detailed-as-usual replies!
> Paul
Re: svn commit: r1002313 -
/subversion/trunk/subversion/tests/cmdline/prop_tests.py
Posted by Daniel Shahaf <d....@daniel.shahaf.name>.
Paul Burba wrote on Tue, Sep 28, 2010 at 18:06:36 -0400:
> On Tue, Sep 28, 2010 at 3:30 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> > pburba@apache.org wrote on Tue, Sep 28, 2010 at 18:38:19 -0000:
> >> + # Run propget -vR svn:mergeinfo and collect the stdout.
> >> + exit_code, pg_stdout, pg_stderr = svntest.actions.run_and_verify_svn(
> >> + None, None, [], 'propget', SVN_PROP_MERGEINFO, '-vR', wc_dir)
> >> +
> >
> > exit_code and pg_stderr aren't checked anywhere.
>
> Neither is pg_stdout...but that entire statement was cruft from an
> earlier version of the test; removed it.
>
:-)
> >> + redir_file = open(redirect_file, 'wb')
> >> + pg_proc = subprocess.Popen(arglist, stdout=redir_file)
> >
> > Shouldn't this use the svntest/ infrastructure? Compare
> > svntest.actions.check_prop().
>
> I didn't use it for two reasons:
I didn't actually mean check_prop() specifically, but rather the
infrastructure it uses --- main.run_command() and main.svn_binary.
> First, svntest.actions.check_prop() only supports finding the props on
> a single path (and as far as I can tell that works fine, no issue
> #3721).
>
> Second, Issue #3721 only occurs when the output of 'svn pg -vR' is
> *redirected to a file* - see
> http://subversion.tigris.org/issues/show_bug.cgi?id=3721#desc1.
> check_prop() is (obviously) all processes and pipes underneath the
> covers, so while it may also be possible to show the bug using it, I
> wrote the test to hew as closely to the actual bug I witnessed as
> possible.
>
Aha, so you're using Popen() directly because you need to have svn's
stdout be a file and not a pipe. I'm a bit surprised that we have a bug
that's that sensitive to trigger, but okay. :-)
> >> + pg_proc.wait()
> >> + redir_file.close()
> >> + pg_stdout_redir = open(redirect_file, 'r').readlines()
> >> +
> >> + # Check if the redirected output of svn pg -vR is what we expect.
> >> + #
> >> + # Currently this fails because the mergeinfo for the three paths is
> >> + # interleaved and the lines endings are (at least on Windows) a mix
> >> + # of <CR><LF> and <LF>. See
> >> + # http://subversion.tigris.org/issues/show_bug.cgi?id=3721#desc1
> >> + unordered_expected_output = svntest.verify.UnorderedOutput([
> >> + "Properties on '" + B_path + "':\n",
> >> + "Properties on '" + C_path + "':\n",
> >> + "Properties on '" + D_path + "':\n",
> >> + " svn:mergeinfo\n",
> >> + " /subversion/branches/1.5.x:872364-874936\n",
> >> + " /subversion/branches/1.5.x-34184:874657-874741\n",
> >> + " /subversion/branches/1.5.x-34432:874744-874798\n",
> >
> > So, 'unordered' also ignores repetitions? (since the last 4 lines
> > appear only once each, rather than three times each)
>
> I think you mean the first 3 lines appear only once, all the other
> lines appear 3 times each (because the test sets the same mergeinfo on
> all three paths and the expected output is for svn pg -v***R***).
>
> But yeah, this test isn't perfect as it would allow repetitions. That
> is the price we pay when using svntest.verify.UnorderedOutput(), which
> is required here because there is no guarantee as to the order in
> which svn pg -vR will report the three paths. I tweaked the test to
> check the expected number of lines in the actual redirected output, so
> that we catch any dups.
>
In short, "Yes" (UnorderedOutput() does ignore repetitions), but thanks
for the detailed-as-usual replies!
> Paul
Re: svn commit: r1002313 - /subversion/trunk/subversion/tests/cmdline/prop_tests.py
Posted by Paul Burba <pt...@gmail.com>.
Hi Daniel,
Comments inline. All fixes mentioned were done in r1002372.
On Tue, Sep 28, 2010 at 3:30 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> pburba@apache.org wrote on Tue, Sep 28, 2010 at 18:38:19 -0000:
>> + # Run propget -vR svn:mergeinfo and collect the stdout.
>> + exit_code, pg_stdout, pg_stderr = svntest.actions.run_and_verify_svn(
>> + None, None, [], 'propget', SVN_PROP_MERGEINFO, '-vR', wc_dir)
>> +
>
> exit_code and pg_stderr aren't checked anywhere.
Neither is pg_stdout...but that entire statement was cruft from an
earlier version of the test; removed it.
>> + # Run propget -vR svn:mergeinfo, redirecting the stdout to a file.
>> + arglist = ['svn.exe', 'propget', SVN_PROP_MERGEINFO, '-vR', wc_dir]
>
> s/.exe// ?
Actually that should ideally be 'svntest.main.svn_binary'. Fixed that.
>> + redir_file = open(redirect_file, 'wb')
>> + pg_proc = subprocess.Popen(arglist, stdout=redir_file)
>
> Shouldn't this use the svntest/ infrastructure? Compare
> svntest.actions.check_prop().
I didn't use it for two reasons:
First, svntest.actions.check_prop() only supports finding the props on
a single path (and as far as I can tell that works fine, no issue
#3721).
Second, Issue #3721 only occurs when the output of 'svn pg -vR' is
*redirected to a file* - see
http://subversion.tigris.org/issues/show_bug.cgi?id=3721#desc1.
check_prop() is (obviously) all processes and pipes underneath the
covers, so while it may also be possible to show the bug using it, I
wrote the test to hew as closely to the actual bug I witnessed as
possible.
>> + pg_proc.wait()
>> + redir_file.close()
>> + pg_stdout_redir = open(redirect_file, 'r').readlines()
>> +
>> + # Check if the redirected output of svn pg -vR is what we expect.
>> + #
>> + # Currently this fails because the mergeinfo for the three paths is
>> + # interleaved and the lines endings are (at least on Windows) a mix
>> + # of <CR><LF> and <LF>. See
>> + # http://subversion.tigris.org/issues/show_bug.cgi?id=3721#desc1
>> + unordered_expected_output = svntest.verify.UnorderedOutput([
>> + "Properties on '" + B_path + "':\n",
>> + "Properties on '" + C_path + "':\n",
>> + "Properties on '" + D_path + "':\n",
>> + " svn:mergeinfo\n",
>> + " /subversion/branches/1.5.x:872364-874936\n",
>> + " /subversion/branches/1.5.x-34184:874657-874741\n",
>> + " /subversion/branches/1.5.x-34432:874744-874798\n",
>
> So, 'unordered' also ignores repetitions? (since the last 4 lines
> appear only once each, rather than three times each)
I think you mean the first 3 lines appear only once, all the other
lines appear 3 times each (because the test sets the same mergeinfo on
all three paths and the expected output is for svn pg -v***R***).
But yeah, this test isn't perfect as it would allow repetitions. That
is the price we pay when using svntest.verify.UnorderedOutput(), which
is required here because there is no guarantee as to the order in
which svn pg -vR will report the three paths. I tweaked the test to
check the expected number of lines in the actual redirected output, so
that we catch any dups.
Paul
Re: svn commit: r1002313 - /subversion/trunk/subversion/tests/cmdline/prop_tests.py
Posted by Paul Burba <pt...@gmail.com>.
Hi Daniel,
Comments inline. All fixes mentioned were done in r1002372.
On Tue, Sep 28, 2010 at 3:30 PM, Daniel Shahaf <d....@daniel.shahaf.name> wrote:
> pburba@apache.org wrote on Tue, Sep 28, 2010 at 18:38:19 -0000:
>> + # Run propget -vR svn:mergeinfo and collect the stdout.
>> + exit_code, pg_stdout, pg_stderr = svntest.actions.run_and_verify_svn(
>> + None, None, [], 'propget', SVN_PROP_MERGEINFO, '-vR', wc_dir)
>> +
>
> exit_code and pg_stderr aren't checked anywhere.
Neither is pg_stdout...but that entire statement was cruft from an
earlier version of the test; removed it.
>> + # Run propget -vR svn:mergeinfo, redirecting the stdout to a file.
>> + arglist = ['svn.exe', 'propget', SVN_PROP_MERGEINFO, '-vR', wc_dir]
>
> s/.exe// ?
Actually that should ideally be 'svntest.main.svn_binary'. Fixed that.
>> + redir_file = open(redirect_file, 'wb')
>> + pg_proc = subprocess.Popen(arglist, stdout=redir_file)
>
> Shouldn't this use the svntest/ infrastructure? Compare
> svntest.actions.check_prop().
I didn't use it for two reasons:
First, svntest.actions.check_prop() only supports finding the props on
a single path (and as far as I can tell that works fine, no issue
#3721).
Second, Issue #3721 only occurs when the output of 'svn pg -vR' is
*redirected to a file* - see
http://subversion.tigris.org/issues/show_bug.cgi?id=3721#desc1.
check_prop() is (obviously) all processes and pipes underneath the
covers, so while it may also be possible to show the bug using it, I
wrote the test to hew as closely to the actual bug I witnessed as
possible.
>> + pg_proc.wait()
>> + redir_file.close()
>> + pg_stdout_redir = open(redirect_file, 'r').readlines()
>> +
>> + # Check if the redirected output of svn pg -vR is what we expect.
>> + #
>> + # Currently this fails because the mergeinfo for the three paths is
>> + # interleaved and the lines endings are (at least on Windows) a mix
>> + # of <CR><LF> and <LF>. See
>> + # http://subversion.tigris.org/issues/show_bug.cgi?id=3721#desc1
>> + unordered_expected_output = svntest.verify.UnorderedOutput([
>> + "Properties on '" + B_path + "':\n",
>> + "Properties on '" + C_path + "':\n",
>> + "Properties on '" + D_path + "':\n",
>> + " svn:mergeinfo\n",
>> + " /subversion/branches/1.5.x:872364-874936\n",
>> + " /subversion/branches/1.5.x-34184:874657-874741\n",
>> + " /subversion/branches/1.5.x-34432:874744-874798\n",
>
> So, 'unordered' also ignores repetitions? (since the last 4 lines
> appear only once each, rather than three times each)
I think you mean the first 3 lines appear only once, all the other
lines appear 3 times each (because the test sets the same mergeinfo on
all three paths and the expected output is for svn pg -v***R***).
But yeah, this test isn't perfect as it would allow repetitions. That
is the price we pay when using svntest.verify.UnorderedOutput(), which
is required here because there is no guarantee as to the order in
which svn pg -vR will report the three paths. I tweaked the test to
check the expected number of lines in the actual redirected output, so
that we catch any dups.
Paul