Hi Luiz, On Fri, Sep 18, 2020 at 3:12 PM Luiz Augusto von Dentz <luiz.dentz@xxxxxxxxx> wrote: > > Hi Miao, > > On Fri, Sep 18, 2020 at 12:44 PM Miao-chen Chou <mcchou@xxxxxxxxxxxx> wrote: > > > > Hi Luiz and Marcel, > > > > Unlike the rest of the existing unit tests in BlueZ, the logic blocks > > tested in test-adv-monitor require dependencies of not only > > src/adv_monitor.c but also all the dependency tree of > > src/adv_monitor.c. The current convention in Makefile.am is to add all > > the extra dependencies one by one. However, the maintenance cost is > > high and not suitable in the case of test-adv-monitor. Therefore, we'd > > like to propose changes in Makefile.am to make the source of > > bluetoothd as a static library and link it for bluetoothd target and > > the unit test target. It would be great if you can provide feedback on > > this idea before the implementation. Thanks in advance! > > Then we should have had the code move to src/shared for unit testing, > but how exactly are you planning to do that? For testing the kernel > interface it normally done via a dedicated tester, but that again can > be done with shared library. > In series https://patchwork.kernel.org/project/bluetooth/list/?series=351021, we introduced some helper functions in adv_monitor.h to perform unit testing and test-adv-monitor to facilitate the unit tests of adv_monitor. We are encountering an expected build check failure on this series. There are two categories in test-adv-monitor, content filtering and RSSI tracking, and content filter is easy to be moved to a standalone shared component while RSSI tracking involves the use of timer, D-Bus method calls and adv_monitor's internal structures, and that makes it strongly coupled with the adv_monitor implementation which require a tree of dependencies apart from adv_monitor. There are two options to resolve the build failures in our case. (1) Reorganize Makefile.am This option is to make the sources (except src/main.c) into a static lib and link this lib in bluetoothd executable target and whichever unit test the sources are required. (2) Create src/shared/am to facilitate helpers and core logic This option is to create a new source under src/shared/ to facilitate helper functions and core logic for src/adv_monitor. The interface of src/shared/am may have the following functions. - Create/destroy functions of struct adv_monitor - Create/destroy functions of struct adv_monitor_device - Helper function for monitor content matching. - Helper function for RSSI tracking However, the logic of RSSI tracking is hard to be ripped off and moved to src/shared/am. One example would be the use of timer in RSSI tracking, and there is currently no previous example of the timer use in the shared library. Series https://patchwork.kernel.org/project/bluetooth/list/?series=351021 is up for review. Our next step here would be ripped off the unit test for now and submit v5 of the series. Once we reach an conclusion on advmon unit test, we can submit a separate series including the refactoring and unit tests. Looking forward to any feedbacks. Thanks! Regards, Miao
