[ wxwindows-Patches-1173507 ] test-cases for the VFS handlers
SourceForge.net
noreply at sourceforge.net
Sun Mar 2 07:56:48 PST 2008
Patches item #1173507, was opened at 2005-03-30 22:20
Message generated for change (Comment added) made by stsp
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=309863&aid=1173507&group_id=9863
Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: Common
Group: bug fix
Status: Open
Resolution: Later
Priority: 5
Private: No
Submitted By: Stas Sergeev (stsp)
Assigned to: M.J.Wetherell (mweth)
Summary: test-cases for the VFS handlers
Initial Comment:
Hello.
Attached are the small fixes for the FindFirst of
wxZipFSHandler.
One patch makes the FindFirst to succeed for the
"zip:/" path
when the directories are allowed. My program does this just
to check whether the handler is installed. FindFirst
succeeds
for "file:/", so I think for "zip:/" it should too.
Another patch makes wxZipEntry::GetName() to return the
absolute path, i.e. the path that starts with "/". Without
this patch, FindFirst fails in a subdirectories of the zip
archieve. But I am not sure if there is no any side-effects
with that patch...
----------------------------------------------------------------------
>Comment By: Stas Sergeev (stsp)
Date: 2008-03-02 18:56
Message:
Logged In: YES
user_id=501371
Originator: YES
But a few things are still pending, like
the leading slash preserving and the tests
for 1189091.
Sorry, I made a real mess in that entry.
But at least I was deleting what was applied.
What isn't deleted, seems to be still pending.
----------------------------------------------------------------------
Comment By: M.J.Wetherell (mweth)
Date: 2008-03-02 18:48
Message:
Logged In: YES
user_id=1262351
Originator: NO
> I don't really know what to do about this patch but it doesn't seem
> useful to keep it here for ~3 years. Is anybody ready to look [again]
> into this?
I applied it and closed it 3 yrs ago.
----------------------------------------------------------------------
Comment By: Stas Sergeev (stsp)
Date: 2008-03-02 18:13
Message:
Logged In: YES
user_id=501371
Originator: YES
This mainly are the tests for the fixes in
[ 1189091 ] FindFirst fixes for local files
https://sourceforge.net/tracker/index.php?func=detail&aid=1189091&group_id=9863&atid=309863
where you have been involved.
When we deal with 1189091, it would be
clearer what to do with this entry.
But right now I can't even say if my
program still depends on these fixes
or not...
By the way, I have a completely rewritten
wxMemoryFSHandler class with support for
FindFirst/FindNext and some other things
needed to create a virtual file system.
I was a bit reluctant to present it here
but if someone thinks it would be a worthy
thing for wx, I may try to.
----------------------------------------------------------------------
Comment By: Vadim Zeitlin (vadz)
Date: 2008-03-02 17:48
Message:
Logged In: YES
user_id=71618
Originator: NO
I don't really know what to do about this patch but it doesn't seem useful
to keep it here for ~3 years. Is anybody ready to look [again] into this?
----------------------------------------------------------------------
Comment By: Stas Sergeev (stsp)
Date: 2005-04-27 21:32
Message:
Logged In: YES
user_id=501371
> now that it has been released. So anything not binary
> compatible will go into the 2.7.x branch when it's
> created in a few weeks time.
That explains the current silence in the tracker, thanks
for clarification. But actually almost all the bugfixes are
binary-incompatible for those who relied on the misbehaveour:)
> I.e. right now FindFirst("wx.zip#zip:*") returns
> "wx.zip#zip:/misc" while FindFirst("wx.zip#zip:misc/*")
> returns "wx.zip#zip:misc/.cvsignore". So we could make
> the former return "wx.zip#zip:misc"
I have no problems with both the above examples of yours.
However it is a bit unclear since the first example doesn't
contain the slash at all. The most important question for me
is what the FindFirst("wx.zip#zip:/misc/*") will return then?
"wx.zip#zip:misc/.cvsignore" or "wx.zip#zip:/misc/.cvsignore"?
----------------------------------------------------------------------
Comment By: M.J.Wetherell (mweth)
Date: 2005-04-27 20:26
Message:
Logged In: YES
user_id=1262351
Hi Stas,
We can only make binary compatible changes to wx 2.6.x
now that it has been released. So anything not binary
compatible will go into the 2.7.x branch when it's
created in a few weeks time.
Also, it's a bit tricky changing the behaviour of
existing functions, since people may be depending on
the old behaviour. Again we'll have a little latitude
on the 2.7 branch. The current functions are a
inconsistent, so it is good that you've looked into it.
I'm wondering whether instead of preserving the slash,
it might be better to normalise the result?
I.e. right now FindFirst("wx.zip#zip:*") returns
"wx.zip#zip:/misc" while FindFirst("wx.zip#zip:misc/*")
returns "wx.zip#zip:misc/.cvsignore". So we could make
the former return "wx.zip#zip:misc", i.e. normalise it
rather than preserve the slash.
Regards,
Mike
----------------------------------------------------------------------
Comment By: Stas Sergeev (stsp)
Date: 2005-04-25 00:20
Message:
Logged In: YES
user_id=501371
Hi.
I made the fixes for the discovered problems, they are here:
https://sourceforge.net/tracker/index.php?func=detail&aid=1189091&group_id=9863&atid=309863
I also updated the patch that is attached here about the
stripping
slash in a zip path. Have you seen it?
With these patches my tests passes, so that I can (hopefully)
complete the missing test-cases.
----------------------------------------------------------------------
Comment By: Stas Sergeev (stsp)
Date: 2005-04-12 20:29
Message:
Logged In: YES
user_id=501371
Yes, it is just a matter of a priority for me - fixing bugs is
kind of more important for my program:) I'll complete the
test-cases, but I'd like to resolve the bugs first.
For example, I finally realized that the current solution with
stripping the leading slash breaks my program sometimes.
We should not just strip-n-forget the slash, as this alters the
returned path for no good reasons, and my program relies
on that.
Attached patch fixes the problem.
----------------------------------------------------------------------
Comment By: Chiclero (chiclero)
Date: 2005-04-12 14:36
Message:
Logged In: YES
user_id=555050
Great thanks, tests are always welcome.
I think the thing to do first is to write tests that
check the recent changes to the zip handler. I can see
you are also adding tests for all the other filesystem
handlers, which is good of you indeed, and also very
welcome.
You could separate the test case into several smaller
functions. Since each test case runs until the first
assertion fails, one problem hides any that come after
it within the same test case.
In general try any extreme cases you think of, then
something 'normal' in the middle.
It's also a good idea to test anything that is known to
have been a problem in the past for regression. e.g.
FindFirst(_T("x.zip#zip:/"));
FindNext();
ChangePathTo(_T("x.zip#zip:/"));
OpenFile(_T("a.txt"));
Tests are always good to have, so thanks again.
Regards,
Mike
----------------------------------------------------------------------
Comment By: Stas Sergeev (stsp)
Date: 2005-04-10 23:16
Message:
Logged In: YES
user_id=501371
OK, lets just re-open this entry then so that it won't left
forgotten.
I'll do the zip tests, but:
1. The tests that were attached are already enough to reveal the
bug in a wxDir (I think) - it would be a good idea to get
that fixed
first.
2. The wxMemoryFSHandler simply asserts on such a tests.
I was planning to implement the proper FindFirst/FindNext for
the memory FS handler anyway, since this is needed for my
program. So I am going to do that first, and this doesn't look
simple at all. Actually I am surprised so many things are not
implemented - it can almost take the one hacking on wx much
more time than writing the program itself:)
----------------------------------------------------------------------
Comment By: Chiclero (chiclero)
Date: 2005-04-10 02:21
Message:
Logged In: YES
user_id=555050
> I just added the test for FindNext(), but removed the tests
> for zip entirely (patch updated).
Thanks for the tests!
> To properly test the zip
> handler, I need some zip file I guess, but where to get it?
We could include a small test zip in with the sources
for you if you like (like testdata.fc is included).
Or you could create one on the fly as a temporary file
or in memory.
----------------------------------------------------------------------
Comment By: Stas Sergeev (stsp)
Date: 2005-04-10 01:35
Message:
Logged In: YES
user_id=501371
> I needed to move the part that matches the root so that
> FindNext() would work.
I just added the test for FindNext(), but removed the tests
for zip entirely (patch updated). To properly test the zip
handler, I need some zip file I guess, but where to get it?
Anyway, since the test fails even for the "file:/", I can wait
before this is fixed first :)
----------------------------------------------------------------------
Comment By: Stas Sergeev (stsp)
Date: 2005-04-10 00:25
Message:
Logged In: YES
user_id=501371
> right.Last() asserts if right is empty.
Hmm, that looks a bit nasty. I'd prefer something like
returning 0
for example, an assert looks too heavy-handed.
Thanks for the explanations.
> I'll have to get to the bottom of that.
OK, I am sorry about that. Something went wrong with my
cppunit. I recompiled it, and now, instead of the crash, it
promptly prints the error messages (and no, the archive
stream is not among the offenders, so it was the false
alarm). But in any case I think the tests are supposed to
pass, all of them. Is this true? But they don't. The output
is attached.
> Some tests would be really good to have.
I did that. The patch is attached. The problem is that
this test fails too:( wxDir needs some patch I suppose.
But right now the code at fault is in a platform-specific
files, which I'd better not touch. So it would be nice if
you can make that test-case to actually work:)
----------------------------------------------------------------------
Comment By: Chiclero (chiclero)
Date: 2005-04-09 15:19
Message:
Logged In: YES
user_id=555050
> > + // a new wxFileSystem object is needed here to avoid
> infinite recursion
> > + wxFSFile *leftFile = wxFileSystem().OpenFile(left);
> Just wondering, what is the scenario of that recursion?
wxFileSystem::OpenFile tries relative paths by
prepending m_Path (which is the value set by
ChangePathTo).
So if you ChangePathTo to inside the zip, then try to
open something with a relative path, when the handler
calls wxFileSystem::OpenFile it prepends m_Path again
and you have an infinite recursion.
So another wxFileSystem object must be used that
doesn't have m_Path set. Let me know if you can see
anything wrong with the fix.
> > - if (right.Last() == wxT('/')) right.RemoveLast();
> > + if (!right.empty() && right.Last() == wxT('/'))
> right.RemoveLast();
> But I thought !right.empty() is implied by
> right.Last()==wxT('/'),
> isnt it?
right.Last() asserts if right is empty.
> > Some tests would be really good to have.
> Yes, but the tests would be good to have working at first:)
> They never worked for me. Below is the stack trace, maybe
> you'll be able to get something out of it.
> There seem to be the memory corruptions in an archive
> streams. When I run the tests without the memory debugger,
> it crashes much later on a regexp tests, but the memory
> debugger njamd, which I used to trust, points to an archieve
> streams.
I'll have to get to the bottom of that. I might need to
ask your help later on to recreate the problem.
Anyway, it's great that you've tried it out, and thanks
for flagging up that problem.
In the mean time, you can to:
$ ./test -l
to see what tests are in there and then just run the
ones that you need:
$ ./test FileSystemTestCase
----------------------------------------------------------------------
Comment By: Stas Sergeev (stsp)
Date: 2005-04-09 12:11
Message:
Logged In: YES
user_id=501371
Hi.
> Applied thanks!
Just a few questions here:
> + // a new wxFileSystem object is needed here to avoid
infinite recursion
> + wxFSFile *leftFile = wxFileSystem().OpenFile(left);
Just wondering, what is the scenario of that recursion?
> - if (right.Last() == wxT('/')) right.RemoveLast();
> + if (!right.empty() && right.Last() == wxT('/'))
right.RemoveLast();
But I thought !right.empty() is implied by
right.Last()==wxT('/'),
isnt it?
> Some tests would be really good to have.
Yes, but the tests would be good to have working at first:)
They never worked for me. Below is the stack trace, maybe
you'll be able to get something out of it.
There seem to be the memory corruptions in an archive
streams. When I run the tests without the memory debugger,
it crashes much later on a regexp tests, but the memory
debugger njamd, which I used to trust, points to an archieve
streams.
Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread -1214567872 (LWP 15897)]
0xb7b6a7db in __gnu_cxx::__pool<true>::_M_reclaim_block ()
from /usr/lib/libstdc++.so.6
#0 0xb7b6a7db in __gnu_cxx::__pool<true>::_M_reclaim_block ()
from /usr/lib/libstdc++.so.6
No symbol table info available.
#1 0x0807bf9f in __gnu_cxx::__mt_alloc<std::string*,
__gnu_cxx::__common_pool_policy<__gnu_cxx::__pool, true>
>::deallocate (this=0xbf935853, __p=0xb4658a40,
__n=8)
at
/usr/lib/gcc/i386-redhat-linux/4.0.0/../../../../include/c++/4.0.0/ext/mt_allocator.h:746
__pool = (
__gnu_cxx::__pool<true> &) @0x82152c0:
{<__gnu_cxx::__pool_base> = {
_M_options = {_M_align = 8, _M_max_bytes = 128,
_M_min_bin = 8,
_M_chunk_size = 4080, _M_max_threads = 4096,
_M_freelist_headroom = 10,
_M_force_new = false}, _M_binmap = 0xb49a4c00, _M_init
= true},
_M_bin = 0xb7f0f180, _M_bin_size = 5, _M_once = 2,
_M_thread_freelist = 0xb473e008,
_M_thread_freelist_initial = 0xb473e000}
__bytes = 32
#2 0x0807c223 in std::_Deque_base<std::string,
std::allocator<std::string> >::_M_deallocate_map
(this=0xbf935984, __p=0xb4658a40, __n=8)
at
/usr/lib/gcc/i386-redhat-linux/4.0.0/../../../../include/c++/4.0.0/bits/stl_deque.h:411
No locals.
#3 0x0807c2a7 in ~_Deque_base (this=0xbf935984)
at
/usr/lib/gcc/i386-redhat-linux/4.0.0/../../../../include/c++/4.0.0/bits/s
tl_deque.h:430
No locals.
#4 0x0807c386 in ~deque (this=0xbf935984)
at
/usr/lib/gcc/i386-redhat-linux/4.0.0/../../../../include/c++/4.0.0/bits/stl_deque.h:715
No locals.
#5 0x0807c3cc in ~Message (this=0xbf935980)
at /usr/local/include/cppunit/Message.h:39
No locals.
#6 0x0807fbb1 in
ArchiveTestCase<wxZipClassFactory>::TestSmartIterator (
this=0xb7f0f000, in=@0xbf935a58) at
./archive/archivetest.cpp:1038
arc = {_M_ptr = 0xb46b0c00}
cat =
{<std::_List_base<Ptr<wxZipEntry>,std::allocator<Ptr<wxZipEntry>
> >> = {
_M_impl =
{<std::allocator<std::_List_node<Ptr<wxZipEntry> > >> =
{<__gnu_cxx::__mt_alloc<std::_List_node<Ptr<wxZipEntry>
>,__gnu_cxx::__common_pool_policy<__gnu_cxx::__pool, true>
>> =
{<__gnu_cxx::__mt_alloc_base<std::_List_node<Ptr<wxZipEntry>
> >> = {<No data fields>}, <No data fields>}, <No data fields>},
_M_node = {_M_next = 0xb46f2508,
_M_prev = 0xb46f2550}}}, <No data fields>}
#7 0x08098c80 in
ArchiveTestCase<wxZipClassFactory>::runTest (this=0xb7f0f000)
at ./archive/archivetest.cpp:473
out = {<wxOutputStream> = {<wxStreamBase> = {
_vptr.wxStreamBase = 0x81b8a48, m_lastcount = 18,
m_lasterror = wxSTREAM_NO_ERROR}, <No data fields>},
m_options = 0,
m_pos = 0, m_capacity = 0, m_size = 0, m_data = 0x0}
in = {<wxInputStream> = {<wxStreamBase> = {
_vptr.wxStreamBase = 0x81b8a88, m_lastcount = 4,
m_lasterror = wxSTREAM_NO_ERROR}, m_wback = 0x0,
m_wbacksize = 0,
m_wbackcur = 0}, m_options = 0, m_pos = 47778, m_size =
47814,
m_data = 0xb465d000 "PK\003\004\024"}
#8 0xb7ed160b in CppUnit::TestCaseMethodFunctor::operator()
(this=0xb4711000)
at TestCase.cpp:32
No locals.
#9 0xb7ebebe9 in CppUnit::DefaultProtector::protect
(this=0xb7f14ad0,
functor=@0xb7f0e304, context=@0xbf935e40) at
DefaultProtector.cpp:15
No locals.
#10 0xb7ecd3ca in
CppUnit::ProtectorChain::ProtectFunctor::operator() (
this=0xb7f0e304) at ProtectorChain.cpp:20
No locals.
#11 0xb7eccf99 in CppUnit::ProtectorChain::protect
(this=0xb7f11600,
functor=@0xbf935eb0, context=@0xbf935e40) at
ProtectorChain.cpp:77
index = -1
#12 0xb7ede742 in CppUnit::TestResult::protect (this=0xb7f11640,
functor=@0xbf935eb0, test=0xb4711000,
shortDescription=@0xbf935ec0)
at TestResult.cpp:178
context = {m_test = 0xb7f0f000, m_result = 0xb7f11640,
m_shortDescription = {static npos = 4294967295,
_M_dataplus = {<std::allocator<char>> =
{<__gnu_cxx::new_allocator<char>> = {<No data fields>}, <No
data fields>}, _M_p = 0xb7bffee4 ""}}}
#13 0xb7ed129a in CppUnit::TestCase::run (this=0xb7f0f000,
result=0xb7f11640)
at TestCase.cpp:27
No locals.
#14 0xb7ed1b87 in CppUnit::TestComposite::doRunChildTests
(this=0xb7f0f080,
controller=0xb7f11640) at TestComposite.cpp:64
index = 1
#15 0xb7ed1abe in CppUnit::TestComposite::run (this=0xb7f0f080,
result=0xb7f11640) at TestComposite.cpp:23
No locals.
#16 0xb7ed1b87 in CppUnit::TestComposite::doRunChildTests
(this=0xb494d040,
controller=0xb7f11640) at TestComposite.cpp:64
index = 1
#17 0xb7ed1abe in CppUnit::TestComposite::run (this=0xb494d040,
result=0xb7f11640) at TestComposite.cpp:23
No locals.
#18 0xb7ee23ee in CppUnit::TestRunner::WrappingSuite::run
(this=0xb494d0e0,
result=0xb7f11640) at TestRunner.cpp:46
No locals.
#19 0xb7ede6a2 in CppUnit::TestResult::runTest (this=0xb7f11640,
test=0xb494d0e0) at TestResult.cpp:145
No locals.
#20 0xb7ee2554 in CppUnit::TestRunner::run (this=0xb4711000,
controller=@0xb7f11640, testPath=@0xbf936164) at
TestRunner.cpp:95
path = {_vptr.TestPath = 0xb7efc608,
m_tests =
{<std::_Deque_base<CppUnit::Test*,std::allocator<CppUnit::Test*>
>> = {
_M_impl = {<std::allocator<CppUnit::Test*>> =
{<__gnu_cxx::new_allocator<CppUnit::Test*>> = {<No data
fields>}, <No data fields>}, _M_map = 0xb46a1320,
_M_map_size = 8, _M_start = {_M_cur = 0xb46a8200,
_M_first = 0xb46a8200, _M_last = 0xb46a8400,
_M_node = 0xb46a132c},
_M_finish = {_M_cur = 0xb46a8204, _M_first = 0xb46a8200,
_M_last = 0xb46a8400, _M_node = 0xb46a132c}}}, <No
data fields>}}
testToRun = (class CppUnit::Test *) 0xb4711000
#21 0xb7ee5e77 in CppUnit::TextTestRunner::run
(this=0xbf93613c, doWait=false,
doPrintResult=true, doPrintProgress=true) at
TextTestRunner.cpp:63
progress = {<CppUnit::TestListener> = {
_vptr.TestListener = 0xb7efca08}, <No data fields>}
testName = Cannot access memory at address 0x0
----------------------------------------------------------------------
Comment By: Chiclero (chiclero)
Date: 2005-04-09 07:15
Message:
Logged In: YES
user_id=555050
Applied thanks!
I needed to move the part that matches the root so that
FindNext() would work.
Some tests would be really good to have. There is a
test suite in tests/filesys/filesystest.cpp which you
could add to.
Regards,
Mike
----------------------------------------------------------------------
Comment By: Stas Sergeev (stsp)
Date: 2005-04-05 21:12
Message:
Logged In: YES
user_id=501371
Hi.
> Maybe wxFileSystem should accept either.
Yes, after some messing around it looks like the only solution.
New patch attached.
----------------------------------------------------------------------
Comment By: Stas Sergeev (stsp)
Date: 2005-04-04 07:27
Message:
Logged In: YES
user_id=501371
> Zips do store paths as relative, so it's better that GetName()
> doesn't make them absolute before returning them.
OK, I see. So it was not the right place to address that issue
then.
> So it looks to me like
> file:wx.zip#zip:/misc
> is actually wrong.
But how about the true relative pathes then? I.e. I can call a
wxFileSystem::ChangePathTo() and expect it to work on zips.
So I think there must be some way to destinguish between
the relative and the absolute pathes.
> Maybe wxFileSystem should accept either.
So the proper place to address that problem might be an
fs_zip handler then, right?
> There does seem to be a slight inconsistency though. For
> example, FindFirst with:
> file:wx.zip#zip:*
> Returns:
> file:wx.zip#zip:/misc
> But:
> file:wx.zip#zip:/misc/*
> then returns nothing
Yes, that exactly is the problem. I think returning
file:wx.zip#zip:/misc is OK. I'd also like all the handlers to
return success for the "/" path.
> The documentation for the filesystem does give this as an
> example:
> file:archives/cpp_doc.zip#zip:reference/fopen.htm#syntax
This should work too unless you call
wxFileSystem::ChangePathTo()
I think. I.e. if the default path will be the root of an
archive,
then both "slashed" and "unslashed" pathes should work,
and we have a backward-compatibility. As soon as someone
changes the path, these two notations became different.
If you don't have an objections to this, I'll try to implement
something like that.
----------------------------------------------------------------------
Comment By: Chiclero (chiclero)
Date: 2005-04-04 05:20
Message:
Logged In: YES
user_id=555050
> One patch makes the FindFirst to succeed for the
> "zip:/" path
> when the directories are allowed. My program does this just
> to check whether the handler is installed. FindFirst
> succeeds
> for "file:/", so I think for "zip:/" it should too.
>
> Another patch makes wxZipEntry::GetName() to return the
> absolute path, i.e. the path that starts with "/".
Zips do store paths as relative, so it's better that GetName() doesn't
make them absolute before returning them. And programs that
extract zips will use the name to create the output files, so making
it absolute will change the behaviour of these existing programs.
> Without
> this patch, FindFirst fails in a subdirectories of the zip
> archieve. But I am not sure if there is no any side-effects
> with that patch...
This works for example:
file:wx.zip#zip:misc/*
There does seem to be a slight inconsistency though. For
example, FindFirst with:
file:wx.zip#zip:*
Returns:
file:wx.zip#zip:/misc
But:
file:wx.zip#zip:/misc/*
then returns nothing (you have to use file:wx.zip#zip:misc/* without
the extra slash).
The documentation for the filesystem does give this as an
example:
file:archives/cpp_doc.zip#zip:reference/fopen.htm#syntax
So it looks to me like
file:wx.zip#zip:/misc
is actually wrong.
Maybe wxFileSystem should accept either.
----------------------------------------------------------------------
You can respond by visiting:
https://sourceforge.net/tracker/?func=detail&atid=309863&aid=1173507&group_id=9863
More information about the wx-dev
mailing list