On Fri, Nov 09, 2012 at 01:01:48PM +0000, Philip Martin wrote:
> Philip Martin <philip.martin_at_wandisco.com> writes:
>
> > Stefan Sperling <stsp_at_elego.de> 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 :)
Received on 2012-11-09 14:15:36 CET