On Mon, Apr 26, 2021 at 4:28 PM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: > I agree that whether or not we should have any tests at all is a matter > of opinion and circumstance. I don't think in general that someone using > some random internal API needs to be checking what is and isn't tested > in that API. > > I do think in this case that it's worth digging a bit further. The APIs > in question are using EINVAL and EISDIR to pass up errors that are had a > 1:1 mapping into the FS historically. > > Are we really better off faking those up, or is some of that perhaps not > used at all? Maybe we'd find that this level of abstraction isn't what > we need, and it could actually be much simpler. I think none of these error functions are being used at all, and I've made a start at removing them in https://github.com/git/git/pull/1012 (see also my discussion with peff.) > It seems to me that a good way to get there is to seek some systematic > ways of focusing review onto various edge cases of this series. I.e. to > begin with having GIT_TEST_REFTABLE pass as noted elsewhere, and in this > case calling attention to some of the underlying assumptions behind this > series. > One of the hardest things I've found about trying to review this has > been closing the gap between things that exist in your mind and commit > messages / code. thanks, that is valuable feedback. > something like this in the commit message (and other applicable commits > etc.) would be *very* valuable: > > In functions such as git_reftable_read_raw_ref() (and ????, are > there more?) we're diligently emulating the documented behavior and > return values of the refs file backend. According to "make gcov" we > can see we don't have coverage for this, in particular the behavior > of EINVAL etc. I haven't done this, because a lot of these considerations are transient, and I'd rather not spend a lot of time documenting what I don't know. > I.e. per [1] once if and when we have GIT_TEST_REFTABLE passing surely > one of the best way to garner feedback on the rest is to discover those > parts (using "make gcov", after running with/without > GIT_TEST_REFTABLE=[true|false]) where we still have outstanding blind > spots between the tests and code. Getting GIT_TEST_REFTABLE=1 passing is the hard part, because it means having to understand exactly how the current code is supposed to work. Once I get to that point (with knowledge being complete and tests passing), it will be easy to document what is happening and why. I was hoping that by posting these series with known test failures, and questions marked "XXX" in reftable-backend.c, I would get feedback from the other people who know exactly how this part of the code works. But from your mail, I get the sense that nobody understands how the whole picture fits together? -- Han-Wen Nienhuys - Google Munich I work 80%. Don't expect answers from me on Fridays. -- Google Germany GmbH, Erika-Mann-Strasse 33, 80636 Munich Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
