No. I think it looks good. I have tested it as well and things still work. On Thursday, March 11, 2021 at 3:39:15 PM UTC-5 Fotis Xenakis wrote:
> On Wednesday, March 10, 2021 at 12:57:01 AM UTC+2 [email protected] > wrote: > >> Thanks for the patch. Let me find some time to properly review the >> changes. It is highly valuable to have some automated tests of DAX window >> functionality. >> >> How confident are you that your changes to the fs/virtiofs/* to make it >> testable with stubs do not break anything? >> > The changes actually touching the manager logic are minimal: mostly the > change from dax_manager::{un}map_ll() accepting chunk numbers to > dax_window_imp::{un}map() accepting bytes. > In addition, testing with the CLI app did not result in any surprises. > > Would it possibly be helpful for me to try breaking down the first patch > into smaller, more manageable ones? > >> >> On Saturday, March 6, 2021 at 10:50:44 AM UTC-5 Fotis Xenakis wrote: >> >>> These add white-box unit tests for the virtio-fs DAX window manager, >>> exercising the manager's internal operations, using Boost.Test. >>> >>> TBH, I am not too confident in these being of great value and also not >>> particularly proud of the result elegance-wise (though this is my first >>> time doing proper automated testing of C++ code, so my expectations >>> might be skewed). In any case, I trust your judgement as to whether >>> these should be merged, modified or not, and would be glad to discuss >>> any point and address and requests. >>> >>> P.S. These are the test cases I had originally tested the implementation >>> with, slightly amended. That testing had been done in isolation though, >>> using a stubbed-out copy of the code, that's why they hadn't been >>> included in the first place. >>> >>> Fotis Xenakis (2): >>> virtio-fs: make DAX manager testable >>> tests: add DAX manager white-box unit tests >>> >>> fs/virtiofs/virtiofs.hh | 9 +- >>> fs/virtiofs/virtiofs_dax.cc | 55 +++--- >>> fs/virtiofs/virtiofs_dax.hh | 90 ++++++--- >>> fs/virtiofs/virtiofs_vfsops.cc | 5 +- >>> modules/tests/Makefile | 6 +- >>> tests/tst-dax.cc | 333 +++++++++++++++++++++++++++++++++ >>> 6 files changed, 444 insertions(+), 54 deletions(-) >>> create mode 100644 tests/tst-dax.cc >>> >>> -- >>> 2.30.1 >>> >>> -- You received this message because you are subscribed to the Google Groups "OSv Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/osv-dev/71e421d0-634f-4b69-aad1-658b07a19b2fn%40googlegroups.com.
