On Thu, Jan 11, 2018 at 01:52:02PM +0200, Amir Goldstein wrote:
> On Thu, Jan 11, 2018 at 1:43 PM, Eryu Guan <eguan@xxxxxxxxxx> wrote:
> > On Sun, Jan 07, 2018 at 08:07:18PM +0200, Amir Goldstein wrote:
> >> Eryu,
> >>
> >> Following patches were used to test overlayfs NFS export.
> >>
> >> Patches 1-5 improve generic/exportfs tests, by adding test coverage
> >> of some directory file handle use cases.
> >
> > open_by_handle test is covering more and more cases, thanks! OTOH, it's
> > getting more and more complicated :)
>
> Yeh, sorry about that ;-)
>
> >
> > But from a quick scan, I think it's time to stop adding new sub-tests to
> > existing tests (maybe we should have done this since last time we added
> > new sub-tests to generic/467), because generic/467 is getting more
> > complex too and makes review harder.
>
> Probably a good idea, but please note that patch 4/7 is fixing a test that was
> not implemented correctly and test 5/7 adds a very minor variant.
I've gone through the first 5 patches now. All look fine except the
comments to patch 1/7.
I think we can add a new test that contains the last part of patch 4/7
("Check non-stale file handles of renamed dirs") and patch 5/7. And
leave the "fix test" part in patch 4/7.
> An option is to split the directory related tests (those that are fixed by 4/7)
> out of generic/467 and then fix them and add the new variant.
This should be fine too, but I slightly tend to not move existing tests,
just add new cases to new tests.
BTW, thanks for the explaining in another reply, that did help.
Thanks,
Eryu
>
> > Perhaps we can factor out some
> > helper functions from generic/467 to common helpers and use them in new
> > tests.
> >
>
> I don't think there is much to factor out, but we can add new tests
> and clone the helpers. Let me know if you think differently after review.
>
> Thanks,
> Amir.
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
