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 -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