On Thu, 2 Jan 2003, William A. Rowe, Jr. wrote: > At 09:56 AM 1/2/2003, Jeff Trawick wrote: > ><[EMAIL PROTECTED]> 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). > > In this case, I agree with Jeff's commit. I also disagree that you can > veto a veto (which I consider Jeff's reversion of this test to be.) > > It was introduced as a pedantic test to try to illustrate to you, Ryan, > the futility of making all platforms behave *identically*. That isn't > reasonable or feasible without adding a ton of extra cycles. > > Apparently this mental exercise didn't have the effects I had hoped.
The "mental exercise" is pointless and not appreciated. I expect more out of this group of people than childish games. And yes, that is harsh, and yes I do mean it. Will, we had a two e-mail conversation where nobody else participated and you decided to back-out your changes. You didn't wait for more comments or try to change my mind. If this test was meant to make me change my mind, it was a stupid way to go about it. Rational arguments almost always work better than trying to play mind games. > Jeff's patch is correct; it isn't APR's job to determine if one can 'open' > a directory as a file. Opening a directory as a file isn't portable, no, > but we can't "help" the programmer on every possible portability issue. You're right it isn't portable. The point of APR is to make it easy to write portable C code. Either document the portability problems, make feature macros, or remove them. Leaving them hidden is _not_ the answer. The original goal of APR was to make all platforms look alike. That doesn't work. So, we began to introduce feature macros that allow the developers to determine which features are available on each platform. The functions that do exist need to work as close to identically as possible on all platforms. This means that error conditions must be testable with a simple macro, and success and failure conditions must be _mostly_ the same. If they aren't the same, that doesn't mean that the test gets removed from the suite though. It means that we need to find a way to write _portable_ code with the failure. That may be a macro test, or it may be lots of small checks in our code. > When the offending code is moved to Win32, yes that will fail. And > then the porter will need to troubleshoot their code, see that they are > trying to 'open' all files *and* sub-directories, and they will have to > modify their code. That's the breaks, we have to draw some line > between enabling portability and enforcing portability. That line is > somewhere about the point that APR stops performing adequately. Which is why we need to have a way to notify people of these problems early. Our docs don't point this stuff out adequately. Should our test suite be part of the docs? No, of course not. But we need to have some way to identify these problems, and the test suite is the best way to do that. Once the problem is identified, we can document things correctly, and we can use XFAIL to notify us when problems like this arise. If you want an example, we now know that we have to document that apr_file_open can be used on directories on some platforms but not others. Great, part of that doc is most likely going to be a list of platforms that it doesn't work on. Now, two years from now, somebody may try to port APR to a new platform. When they do, won't it be nice to know that we need to add their platform to the list of platforms that can't use apr_file_open on directories? I think it would be great to be able to do that. The way to do it is to have this stuff tested in the test suite, and put a note that says "If you failed this test, you can just document that fact". Ryan
