apr-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From <...@apache.org>
Subject Re: cvs commit: apr/test testfile.c
Date Thu, 02 Jan 2003 16:22:54 GMT


On 2 Jan 2003, Jeff Trawick wrote:

> <rbb@apache.org> writes:
>
> > Guys, this is ridiculus.  If a test fails, the correct approach is not to
> > remove the test from the test suite.  The hole point of the test suite is
> > to ensure that APR works the same on _ALL_ platforms.  If it doesn't, then
> > it is just as hard to write portable programs with APR as without.
> >
> > There are two options when a test fails.  1)  Fix the bug in the APR code.
> > 2)  Introduce a feature macro that allows programmers to determine if a
> > feature exists with #ifdefs.  Removing a test is _not_ an option.  It
> > weakens APR's portability.
> >
> > Plus, not long after Will added this test, I posted to the list remarking
> > that it was a problematic test and looking for possible solutions.  We
> > should at least have a conversation over how we want to deal with this.
> >
> > -1 for removing this test.
>
> What is the precise technical reason for your veto?
>
> I think it is not a valid property to test in an APR test suite
> (outside the scope of APR) and it is harmful to leave it there (not
> portable).

This isn't outside the scope of APR.  You have an API that is exposed to
our users.  That API needs to work the same on all platforms or you will
be asking users to remember "I can apr_file_open directories on Unix, but
not Windows", "I can open files without APR_READ and APR_WRITE on Windows,
but now Unix".  The whole goal of the test suite is to ensure that our
APIs are portable and work the same on all platforms.  By the time the
test suite is done, it should be possible to completely re-implement APR
without duplicating code, and know for a fact that you have an
implementation that will allow programs to be ported to new platforms
easily and without worrying abut platform differences.  That goal isn't
possible if we don't define what features are and are not supported by
APR.

The test that you removed shows us a place where a function works
differently on Unix and Windows.  By removing the test, you have done 2
things.  You have hid this problem from people making it harder to fix,
and you have removed a perfectly valid test.  The results may not be what
you want, but the results are valid.  Unix is failing a test.

There are two ways to resolve this bug.  You can create a feature macro
APR_FILE_OPEN_DIRS, which means that apr_file_open will work on
directories, or you can make apr_file_open fail to open directories on
Unix.  In the meantime, leave the failure in the test suite.  It is
highlighting a problem.

No platform that fails a test is allowed to just remove that test.
OtherBill tried to do that this weekend because Windows allows you to open
a file without either APR_READ or APR_WRITE.  That test showed us a
difference between Windows and Unix that would have potentially tripped up
an APR programmer.  We fixed that bug in the code, and the test passes
now.  This test is just one more bug that needs to be fixed.

As for this being a platform difference that we can't control, that is not
true.  We can fix this bug, it is just that the two solutions that we
currently know about are too heavy-weight for us to use them.  We could
stat the file in Unix to determine if it is a directory, or we could try
to open the directory on Windows if opening the file fails.

So, the precise technical veto is that this test is 100% valid, and by
removing it you have made APR less portable and harder to use.  Please add
it back.

Ryan


Mime
View raw message