DevHeads.net

Review Request: Test also the prefix value

Review request for kdelibs and David Faure.

Description
We need to test if there is a prefix value because the file name can be
null and the prefix can have a value and in this case we don't reset n.

It's the last part to fix the bug 258737

This addresses bug 258737.
<a href="http://bugs.kde.org/show_bug.cgi?id=258737" title="http://bugs.kde.org/show_bug.cgi?id=258737">http://bugs.kde.org/show_bug.cgi?id=258737</a>

Diffs
kdecore/io/ktar.cpp 9c3a010

Diff: <a href="http://git.reviewboard.kde.org/r/104068/diff/" title="http://git.reviewboard.kde.org/r/104068/diff/">http://git.reviewboard.kde.org/r/104068/diff/</a>

Testing

Thanks,

Mario Bensi

Comments

Re: Review Request: Test also the prefix value

By Commit Hook at 02/27/2012 - 11:36

This review has been submitted with commit 6e0d694b302b57883ed79a3b47c9d60bea20dc8e by Mario Bensi to branch KDE/4.8.

- Commit Hook

On Feb. 24, 2012, 5:31 p.m., Mario Bensi wrote:

Re: Review Request: Test also the prefix value

By Frank Reininghaus at 03/03/2012 - 08:05

Hi,

Am 27. Februar 2012 17:36 schrieb Commit Hook:
The new unit test which you included in this commit fails for me
because the owner of a file in the archive does not match the
expectation:

FAIL! : KArchiveTest::testTarDirectoryForgotten() Compared values are
not the same
Actual (listing[ 9]): mode=40777 user=kde-4.8 group=users
path=Test/qt-jambi-qtjambi-4_7/examples/generator/trolltech_original/java/com/trolltech/examples/generator
type=dir
Expected (QString("mode=40777 user=nef group=users
path=Test/qt-jambi-qtjambi-4_7/examples/generator/trolltech_original/java/com/trolltech/examples/generator
type=dir")): mode=40777 user=nef group=users
path=Test/qt-jambi-qtjambi-4_7/examples/generator/trolltech_original/java/com/trolltech/examples/generator
type=dir
Loc: [/home/kde-4.8/kde/src/KDE/kdelibs/kdecore/tests/karchivetest.cpp(629)]

It looks like I'm not the only one getting this failure:

<a href="http://my.cdash.org/testDetails.php?test=8836261&amp;build=305462" title="http://my.cdash.org/testDetails.php?test=8836261&amp;build=305462">http://my.cdash.org/testDetails.php?test=8836261&amp;build=305462</a>
<a href="http://build.kde.org/job/kdelibs_KDE-4.8/265/testReport/%28root%29/TestSuite/kdecore_karchivetest/" title="http://build.kde.org/job/kdelibs_KDE-4.8/265/testReport/%28root%29/TestSuite/kdecore_karchivetest/">http://build.kde.org/job/kdelibs_KDE-4.8/265/testReport/%28root%29/TestS...</a>

Could you have a look at that issue?

Thanks,
Frank

Re: Re: Review Request: Test also the prefix value

By Mario Bensi (Nef) at 03/03/2012 - 09:25

Hi,

You rigth, sorry for the disturbance, I have fixed the problem and it's pushed

<a href="http://commits.kde.org/kdelibs/3306c442f9f8be2f99251ba401ed9259200a02ee" title="http://commits.kde.org/kdelibs/3306c442f9f8be2f99251ba401ed9259200a02ee">http://commits.kde.org/kdelibs/3306c442f9f8be2f99251ba401ed9259200a02ee</a>

can you test if it's fixed for you too ?

Mario

Le Saturday 03 March 2012 14:05:37 Frank Reininghaus a écrit :

Re: Re: Review Request: Test also the prefix value

By Frank Reininghaus at 03/04/2012 - 05:56

Hi,

Am 3. März 2012 15:25 schrieb Mario Bensi (Nef):
yes, the modified test passes passes here. Thanks for the quick fix!

Regards,
Frank

Re: Review Request: Test also the prefix value

By Kevin Ottens at 02/27/2012 - 03:04

Ship it!

I still don't have the fine knowledge of tarball structure. :-)

But this patch looks sane and coherent with previous reviews, if it helps supporting more files, go for it.

- Kevin Ottens

On Feb. 24, 2012, 5:31 p.m., Mario Bensi wrote: