cederom commented on PR #2931: URL: https://github.com/apache/nuttx-apps/pull/2931#issuecomment-2580204867
Thank you @GUIDINGLI for the feedback :-) > > @cederom: Thank you @txy-21 good idea, it should keep things clean, on the other hand folks may search for existing tests in subdirectories :-P PR needs a rebase. Then we can see the CI build results :-) > > Some questions: > > * do we / should we want to resemble `nuttx.git` organization with these tests? > > @GUIDINGLI: Not exactly the same layout with nuttx.git, because something is not suitable, like nsh_test (if we have in the feature), lvgl... So we prefer the feature category: drivers libc fs rpmsg ... okay :-) > > * `himem_test` and `memtest` are moved to `arch/` but `mm` is not, why? Maybe we want dedicated `memory/` section for all memory related tests? > > Because himem_test not the common mm test, it depends on the specific arch chip. And either 'mm' or 'arch' is ok for me, we can change to 'mm' if you insist. If its more architecture specific then `arch/` is better location, thanks :-) > > * If we want to resemble `nuttx/` organization do we want `drivers/` for drivers test (in place of `driver`)? > > yes, we will change it to 'drivers' thanks :-) > > * `cmocka` does not really tell what category is tested under? Why it is placed under `fs/`? Is it the cmocka framework test or cmocka is used to test something else? If so the tests should be named by their function not a test tool used? See `mm/cmocka` comment below, if tests are using cmocka framework and tests inside are self-explanatory then okay :-) > > Yes, the 'mm/cmocka' will be confuse, we can change to 'mm/basic' ? And we don't want to modify the test code in this PR, just move the folder and classify them. The `mm/cmocka` is also fine as it shows the Memory Management has dedicated tests performed with cmocka. It can be named `mm/generic` or `mm/standard` or whatever fits best, maybe `mm/basic_tests` is better than `basic` as it does not remind about the BASIC programming language :D What other folks think about this? :-) > > * before we had `testuites/kernel/fs/*` now it is renamed to `fs/cmocka/*` > same will 'mm/cmocka' change to `fs/basic/*` ACK > > * what happens to `fff`? It is renamed to `fs/cmocka/` but no reason why and what happens with `fff`. Anyone using `fff`? If so they will see things just disappeared with no info in the commit message. > > We think the fff is none use it, so we decide remove it. Will split to another PR to discuss this. I think the mention of removing `fff` should be in this PR (commit mesage) because this PR and commit removes it.. or whatever place / PR touches the code. Just to let folks know what happened and why it is not here anymore :-) > > * what is the `fdsantest` test for? do we want to update description too? > The test case of tools fdsan, for the file descriptor used after free. The description will update in another PR. ACK > > * do we want `nand_sim` in `fs/` or `drivers/`? does it operate on driver or filesystem level? > Good find, should move to drivers. thanks :-) > > * do we want `sd_bench` and `sd_stress` in `fs/` or `drivers/`? does it operate on driver or filesystem level? > Aslo move to drivers. thanks :-) > > * do we want `libc` in `libs/libc/libc_test` to resemble `nuttx` layout? > > * do we want `arch_libs` in `libs/arch_libs`? > > * do we want `fmemopen` in `libs/libc/stdio/fmemopen`? `libs/libc/fmemopen` is also fine. > > * do we want `scanftest` in `libs/libc/stdio/scanftest`? `libs/libc/scanftest` is also fine. > > Probably not, we named the folder base on feature category, how do you think ? I think `libs/libc` will provide more space for other libs in `libs/` too. What other people think about this? :-) > > * `mm/cmocka` is similar to `fs/cmocka`.. I assume these are `mm` tests performed with use of cmocka framework and test inside will be self-explanaotory? Still it would be nice to know what tests are included and performed under cmocka? > > 'mm/cmocka' change to `fs/basic` will be OK ? And the cmocka-based testcases are just cases. We should be classify them with features not the case-based ? Yes, this is the point, cmocka name can be used to show cmocka framework is used for testing, but when looking inside tests should be self-explanatory to easily know what is tested under.. maybe some description like "generic feature tests performed with cmocka framework" would be helpful.. I think it works like this right now? :-) > > * should `monkey` go to `drivers/monkey` instead of `mm/monkey`? > Agree & Accept. thanks :-) > > * should `resmonitor` go to `os/resmonitor`? is `mm/` a good selection? > So we start a 'os' folder, as the stress-likely test ? How about 'stress' > > * should `stressapptest` go to `os/stressapptest`? > OK, But the 'monkey' should also the go to 'os/monkey' or 'stress/monkey' ? because they are all the stress test. I think that `os/` is more generic and more OS specific tests can land there including stress-testing. If you like `stresstesting` can be a dedicated separate category why not :-) Monkey test is undocumented and it seems to test UINPUT so probably `drivers/` would be good category as agreed above before? > > * I can see that `fff` again is removed and becomes part of `net/`. why? > No, we remove it. As said before will start a new PR, to see opinions from others. Okay, it should not be touched here in that case :-) > > * `testsuites/kernel/socket` becomes `net/cases`.. shouldn't it become `net/cmocka` to align with other cmocka tests? > Also change to `net/basic` Or `net/cmocka` to be discussed with other devs, whatever name fits best :-) > > * do we want `rpmsg` to go to `fs/rpmsg` or `net/rpmsg`? > > No, rpmsg is a independent feature, is for the AMP IPC ACK :-) > > * do we want `atomic` to go to `os/atomic` or `sched/atomic` is okay? > > Prefer `sched/atomic` Okay :-) If the `sched/` is better place then be it. This is why I considered dedicated `os/` category for all generic OS features testing :-) > > * why `sched/cmocka_mutex` has dedicated category not `sched/cmocka` as other cmocka tests? > > * the same as above for `sched/cmocka_pthread` why not just case for `sched/cmocka`? > > * the same as above for `sched/cmocka_syscall`? > > * the same as above for `sched/cmocka_time`? > > The folder name should a feature, not a 'cmocka-based' folder name I think. somewhere named cmocka because we don't want the same name, so change to 'baisc'. 'sched/sched' change to 'sched/basic' @txy-21 Okay so the pthread, syscall, time, etc, should be merged into one single `sched/cmocka` ? > > * do we want `cpuload` in `os/cpuload` or `sched/` is better location? > > * do we want `getprime` in `os/getprime` or `sched/` is better location? > > * do we want `smp` in `os/smp` or `sched/` is better location? > > * do we want `timerjitter` in `os/timerjitter` or `sched/` is better location? > > Prefer sched ACK :-) I am curious of other folks ideas here? shed vs os :-) > > * Regarding the `os/` test location it may contain generic (RT)OS tests while `sched/` test would be dedicated to NuttX scheduler.. OS test may contain also other tests like `os/ostest` etc. > > 'os' is too big, all the testcase can after os, because we are doing a OS ^_^ Yeah but it seems intutitive to have all generic tests over there.. but I am not enforcing anything.. if other folks also think that wat let it be `sched/` :-) > > * It seems that not all tests are moved? > > The fist version only change the 'apps/testing'. ACK :-) Thank you :-) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
