Re: svn commit: r1002313 - /subversion/trunk/subversion/tests/cmdline/prop_tests.py
pbu...@apache.org wrote on Tue, Sep 28, 2010 at 18:38:19 -: + # 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 CRLF 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
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: pbu...@apache.org wrote on Tue, Sep 28, 2010 at 18:38:19 -: + # 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 CRLF 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
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: pbu...@apache.org wrote on Tue, Sep 28, 2010 at 18:38:19 -: + # 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 CRLF 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
