On Fri, Nov 09, 2012 at 01:01:48PM +0000, Philip Martin wrote: > Philip Martin <[email protected]> writes: > > > Stefan Sperling <[email protected]> writes: > >> > >> Can I just commit that to the 1.7.x branch as obvious fix? > >> > >> Index: subversion/tests/cmdline/patch_tests.py > >> =================================================================== > >> --- subversion/tests/cmdline/patch_tests.py (revision 1407431) > >> +++ subversion/tests/cmdline/patch_tests.py (working copy) > >> @@ -3939,7 +3939,7 @@ def patch_target_no_eol_at_eof(sbox): > >> "context", # no newline at end of file > >> ] > >> expected_output = [ > >> - 'U %s\n' % os.path.join(wc_dir, 'A/mu'), > >> + 'U %s\n' % os.path.join(wc_dir, 'A', 'mu'), > >> 'U %s\n' % os.path.join(wc_dir, 'iota'), > >> ] > > > > The same code, without the above patch, seems to PASS on trunk. Why does > > the branch need different code? Does the testsuite have some path > > normalisation that corrects/hides the bug? > > > > If that patch is the correct solution I think you should commit to trunk > > and backport. > > Ah! trunk uses the sbox.ospath which handles "foo/bar" on Windows, I > didn't spot that when I compared the code. So we could fix it with your > patch above or by switching to sbox.ospath to make the code look like > trunk.
Yes. This backport change was made because of a text conflict that happened because gstein switched almost the entire test suite to use sbox.ospath() on trunk. Whether to patch it to use sbox.ospath() or keep using os.path.join() can be argued either way. It doesn't matter. I'd prefer to just commit this little patch. I wouldn't want to have mixed use of sbox.ospath() and os.path.join() idioms in a single test, and switching the entire test over to sbox.ospath() is additional and unnecessary work. Anyway, this shows that switching to os.ospath() on trunk was a good thing because it avoids this kind of mistake in the first place :)
