-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Proposed changes to split tests into separate binaries for efr32 [Version 2] #33790
Proposed changes to split tests into separate binaries for efr32 [Version 2] #33790
Conversation
…to use glob, and updated the test runner to use the flash_image parameter for directories
examples_plat_dir = "${chip_root}/examples/platform/silabs/efr32" | ||
examples_common_plat_dir = "${chip_root}/examples/platform/silabs" | ||
#efr32_project_dir = "${chip_root}/src/test_driver/efr32" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bunch of stuff needs importing into here since it is used by silabs_executable
. Eventually I'll find a cleaner way to do this.
@@ -68,7 +68,7 @@ template("silabs_executable") { | |||
] | |||
copy(flashing_runtime_target) { | |||
sources = flashing_script_inputs | |||
outputs = [ "${root_out_dir}/{{source_file_part}}" ] | |||
outputs = [ "${root_out_dir}/${target_name}--{{source_file_part}}" ] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "copy" action only needs to be done once per build, however now that silabs_executable
is being called many times it ends up trying to copy these same python files repeatedly which causes an error. I could not find a clean way of making it only copy once, so as a temporary fix I changed the names of the files to be something unique. However ultimately this is not what we want, so I'll need to circle back and fix this properly.
output_dir = root_out_dir | ||
} | ||
} | ||
|
||
pw_test(_test_name) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The chip_test_suite
template makes calls to pw_test
(part of pigweed) for each test source instead of calling chip_test
(part of our codebase). Currently chip_test
is only being used by chip_test_suite_using_nltest
, so I presume it has not been updated to work with PW unit testing. If chip_test
were updated to support PW, we could make it so that chip_test_suite
calls chip_test
(instead of pw_test
), and then I could probably move all of the silabs_executable
stuff and the efr32 stuff into chip_test
rather than the foreach loop of chip_test_suite
, which would be slightly cleaner.
REPLACED BY: 34769
The approach here is to move the
silabs_executable
target intochip_test_suite
. The advantage is that we don't need to update anything if we add a new directory of tests, since it will add new efr32 binary targets each timechip_test_suite
gets invoked. The disadvantage is that it moves a lot of efr32-specific stuff intochip_test_suite
, which may not be desirable.This version creates a binary for each test source (e.g.
src/messaging/tests/TestExchange.cpp
). It can easily be modified to create a binary for each suite (e.g.src/messaging/tests
) just by moving the location of the silabs_executable target outside of theforeach(_test, invoker.test_sources)
loop and making a few other minor changes. However in the interest of allowing for very large tests or for test directories with many test sources in them, I decided to split it per test source.So far I have not gotten this to build successfully, probably due to
nl_test_service
being defined in the wrong place. Solving this issue is the next step, however I wanted to stop and get some feedback about the overall approach before going further.(This is a work in progress, so please ignore any commented-out code and print statements.)