Code review comment for lp:~songofacandy/bzr/fix-523746-dev

Revision history for this message
methane (songofacandy) wrote :

I'm ashamed to leave this so long.

> > Vincent Ladeuil wrote on 2010-04-21 :
> > > That's the main point I dislike with the current tests, they don't
> exercise
> > > subprocess.Popen, but I don't have an external diff tool I can call
> > available
> > > on all platforms.
> >
> > I've fixed that in this branch: lp:~garyvdm/bzr/523746-difftool-file-names
> >
> > Those tests fail in wine for both Vincent's and INADA's branches.
>
> I'll see the branch later.

Your test fails because my branch changes filename for it safely write
to the filename and pass the filename to subprocess.

I split filename creation from DiffFromTool._write_file and wrote test for it.
Please see it.

I've wrote test executing DiffFromTool but it doesn't tests real problem
because overriding sys.getfilesystemencoding() doesn't changes
Py_FileSystemDefaultEncoding in CPython core.

« Back to merge proposal