[ZODB-Dev] TmpStore missing loadBefore

[Tim Peters]

[Sidnei da Silva]
> I would expect a 2.8.1 soon, as in no longer than 3 weeks from now tops.
> There's this bug which I would consider 'serious' and another issue with
> the last-minute inclusion of BTreeFolder2 breaking CMF 1.5. IANRM though.

I haven't seen anything about 2.8.1 yet from its release managers
(presumably still Andreas and Brian).  Andreas seems to be concentrating on
Zope 2.7.7 over the next few weeks.

...

[Tim Peters]
>> Ah, maybe that's where we're getting off track.  Not really; TmpStore
>> and FileStorage both write pickles to files, and both keep a kind of
>> index structure, but those are shallow similarities.  TmpStore is about
>> 60 lines of code total, while FileStorage is almost 2100 in
>> FileStorage.py alone (not counting support modules only FileStorage
>> uses); because TmpStore _must_ defer to the underlying storage for most
>> operations, it couldn't even usefully take more from FileStorage.

> But, but, but... even for those 60 lines, would it make sense to factor
> out the code from FileStorage so that they could share just this small
> bit of common functionality?

It doesn't seem so to me, but I don't know exactly what you have in mind.
For example, what good would it have done for the loadBefore() bug?  The few
lines of TmpStore code that store strings in a file had nothing to do with
that; and inheriting loadBefore() from FileStorage would have been dead
wrong, so there was no (correct) opportunity for sharing there.

[Jim Fulton]
> ... I still do think that TmpStore *is* an internal implementation
> detail and that tests of actual public functionality are more important.
> I've seen lots of cases in the past where internal components were
> tested directly with the following disadvantages:
>
> - test effort was wasted unnecessarily when implementation decisions were
>    changed,
>
> - It was often hard to tell which test tested essential functionality
>    and which tested implementation accidents, this making refactoring
>    more difficult.
>
> I think tests should be writted for real external apis.  I think testing
> internal implementation details is almost always waste of scarce
> resources.

That's "general principle".  I'm responding to Sidnei's specific complaint:

    This is not the first time I've hit a (shallow) bug in TmpStore that
    would just be triggered by a fairly big or long
    subtransaction/savepoint.

If he wants to volunteer tests to prevent that from happening again, I
remain all in favor of it.  Although, as with the test I added for the
loadBefore() bug, I think it would be more_natural to write tests that
indeed trigger "a fairly big or long subtransaction/savepoint" rather than
poke around in TmpStore internals.