Skip to content
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

[tools] Compiling tests produces an empty config for the test sources #2397

Closed
AlessandroA opened this issue Aug 9, 2016 · 12 comments
Closed

Comments

@AlessandroA
Copy link
Contributor

If you have the following setup:

~/some_folder/
├── mbed_app.json
└── TESTS
    └── test_group
        └── test_case
            └── main.cpp

and try to compile the tests with:

$ mbed test --compile -m K64F -t GCC_ARM -n tests-test_group-test_case

The file ~/some_folder/TESTS/test_group/test_case/main.cpp will see an empty config (empty mbed_config.h file), despite mbed_app.json being there.

Proposed solution

I think the issue is in tools/toolchains/__init__.py. When multiple sources of config data are possible, and all of them are empty/non-existent, the data structure holding the configs looks like this: ({}, {}, ..., {}). This line checks if there are no configs as follows:

if config:
    # there are configs
else:
    # there is none

but with the tuple it doesn't work. A suitable check could be:

if any(config):
    # there are configs
else:
    # there is none

(Not tested with all possible config combinations, especially when there is only 1 config source. In that case we should make sure that it's still a tuple, with one element.)

@sg- @geky

@AlessandroA
Copy link
Contributor Author

Also, if I fix this locally, I notice the top-level config (mbed_app.json) is not included in the compilation of the test case. Probably a new issue to file? @sg-

@bridadan
Copy link
Contributor

bridadan commented Aug 9, 2016

@AlessandroA That's an interesting point about the top-level config not being included in the test case compilation. I would actually expect this to be the case, as the application shouldn't really affect the compilation of the test cases (and in fact we still don't really have a good way of allowing applications and tests to live side-by-side). Can I ask what is your use case?

@AlessandroA
Copy link
Contributor Author

@bridadan Yes it's true, a top-level mbed_app.json should not be seen automatically by test cases as that might be an app-specific config file.

My use case is one where I have the following layout:

~/uvisor-tests
├── mbed-os
├── common-utilities
└── TESTS
    └── test_group
        └── test_case
            └── main.cpp

I currently run mbed test in the uvisor-tests folder above. This in turn builds the mbed-os and common-utilities libraries, and then builds the test_case, linking those libraries in.

What I would like to achieve is to have a config file that applies globally to all test cases and to the libraries as well. Applications get this for free because the app is the root of the project, and the mbed_app.json file applies to everything in the tree below.

For tests, I'm forced to have an mbed_app.json file in the top-level folder, which applies to the mbed-os and common libs, and then an additional mbed_app.json (a copy of the other one) in the test case folder, which applies to the test case itself.

Do you suggest a different setup? Or is this a tool limitation that we need to fix?

~/uvisor-tests
├── mbed-os
├── common-utilities
├── mbed_app.json                       * applies to mbed-os and common-utilities
└── TESTS
    └── test_group
        └── test_case
            ├── main.cpp
            └── mbed_app.json           * applies to test_case

@0xc0170

@bridadan
Copy link
Contributor

@AlessandroA Is this configuration only valid when common-utilities is present? In that case, could you use and mbed_lib.json instead inside of that directory? Here's the docs page.

Or is that too limited? If so, could you detail what config options need to be set in your use case that can't be done with an mbed_lib.json and an mbed_app.json?

@AlessandroA
Copy link
Contributor Author

@bridadan Yes common-utilities will always be there. What troubled me is that I need to duplicate the mbed_*.json config files (same content, which is more or less this one).

Since a test case is just a glorified application (it generates a binary), I thought there was a way to apply a config file to all source code (which is true for apps).

I'd also be happy to have a config file that magically applies to all test cases in the TESTS folder, but that's a different thing 😄

@bridadan
Copy link
Contributor

@AlessandroA thanks for clarifying! In the case you described, it sounds like placing that config in an mbed_lib.json in common-utilities should satisfy your needs. That should be applied to every test binary. Or am I missing something here?

@AlessandroA
Copy link
Contributor Author

AlessandroA commented Aug 11, 2016

@bridadan It doesn't work because I'm using target_overrides, which is only allowed at the app level.

But even if I only define macros in common-utilities/mbed_lib.json, the file is not -included in the test case sources. I think this is a bug. The issue I reported originally here is a first step to unveil it.

So to sum up:

  • The current code has what I believe is a bug: It generates an empty config file specifically for the test case sources (see first comment here to reproduce), even if other config files are present in the source tree.
  • If that is fixed, no config file is generated at all for the test case sources, and no -include option is added to the test case sources build command, even if other config files are present in the source tree.

@bridadan
Copy link
Contributor

@AlessandroA The doc I posted suggests that target_overrides is supported in mbed_lib.json, are you finding this to not be the case?

It's possible that the way we are building tests is causing the build system to not pickup the config.

@bogdanm we currently build the entire code base (excluding tests) as .o files, put that in a directory, and include that build directory as a "source" folder. We then add the test folder containing the entry point to the test as a "source" folder as well and build to a totally separate build directory. Would the inclusion of the built library cause the config to not be picked up? Here's the build step described with paths:

/myproject/.build/tests/<TARGET>/<TOOLCHAIN>/[build the entire codebase as a non-static library here]

Now call what is essentially this for all of the tests in the code base:

mbed compile --source /myproject/.build/tests/<TARGET>/<TOOLCHAIN> --source TESTS/testgroup/testcase --build /myproject/.build/tests/<TARGET>/<TOOLCHAIN>/TESTS/testgroup/testcase

Perhaps the use of the shared OS build is what's causing the config to be lost?

@bogdanm
Copy link
Contributor

bogdanm commented Aug 16, 2016

Perhaps the use of the shared OS build is what's causing the config to be lost?.

I believe so, yes. The config system needs to have access to the various mbed_lib.json files that are found during the resource scanning process in order to be able to generate mbed_config.h. If none of the source directories have any mbed_lib.json files, no config will be generated. There are a few ways to handle this:

  1. build from source
  2. propagate configuration data to the binary build

1 would be ideal, but I imagine that it'd increase the build time by an unacceptable amount, so we're probably stuck with 2. 2 is also a bit tricky, because you need the configuration data of all the libraries that were compiled to generate the binary build (so you can't assume that there's a single mbed_lib.json file that you'll find while compiling the libraries and that needs to be copied in the same directory as the object files, you need to handle more than one mbed_lib.json). Where's the code that we use to build all the .o files?

@AlessandroA
Copy link
Contributor Author

@bridadan Sorry I missed your questions.

@AlessandroA The doc I posted suggests that target_overrides is supported in mbed_lib.json, are you finding this to not be the case?

I am seeing this (mbed-cli v0.9.1):

ConfigException: Target override 'target.extra_labels' in 'library:common[K64F]' is only allowed at the application level
[ERROR] Target override 'target.extra_labels' in 'library:common[K64F]' is only allowed at the application level

@bridadan
Copy link
Contributor

@AlessandroA ah dang. I found the line of code that's issuing this error: https://github.com/ARMmbed/mbed-os/blob/master/tools/config.py#L456

So it looks like we intentionally don't allow setting extra_labels in libraries. If you need to use this option then you may have to duplicate the app config :/

@sg-
Copy link
Contributor

sg- commented Sep 19, 2016

@AlessandroA Is this resolved?

@sg- sg- closed this as completed Jan 16, 2017
artokin pushed a commit to artokin/mbed-os that referenced this issue Aug 17, 2020
…..48609ae

48609ae Merge branch 'release_internal' into release_external
62d8586 Ignore ns_monitor when receiving Ack (ARMmbed#2417)
3323f36 Add support for Ethernet RA dns configuration
d8e7d40 Iotthd 4239 (ARMmbed#2414)
b46f3c6 add empty function for ws_stack_info_get
fc97980 Changed RADIUS shared secret length to 16-bit value
f827ffc Added information API to Wi-SUN and border router
8f1f9d5 EDFE error handling update
51bf94e Fix adaptation interface unit tests (ARMmbed#2409)
0860b57 FHSS_WS: Fixed reading unicast remaining slots (ARMmbed#2408)
4d8c03b Border Router RADIUS client basic authentication functionality (ARMmbed#2406)
fbfada9 Adaptation IF: Allocate fragmentation buffer only if needed (ARMmbed#2407)
66f1bff Added storing of PAN version to NVM on BR
89826ce Iotthd 4224 (ARMmbed#2403)
3fc1ae2 BR EUI-64 is now selected for 4WH using PMKID on 4WH Message 1
af8438e Timing tool traces (ARMmbed#2401)
7938795 Fixed new PD data request for check if EDFE exchange is active.
85ab8fd Extented Frame exchange support
86b1f27 Merge pull request ARMmbed#2399 from ARMmbed/IOTTHD-4220
fed69e0 Add missing test method to ws_empty_functions
6b58e26 Add EDFE mode to Socket API setsockopt
1283077 Test API to adjust 6LoWPAN fragmentation MTU size (ARMmbed#2398)
e787874 Init MAC MTU size based on driver MTU size (ARMmbed#2397)
bf8e89e Ignore neighbors using unsupported channel function (ARMmbed#2395)
1c263fd FHSS exclude channel usage from mask and Brazilian Domain support
841dcbe MAC: Configurable data whitening (ARMmbed#2393)
9a10d66 Fix global address detection (ARMmbed#2392)
f27fe86 Corrected network name and PAN ID change on auth start
bcce0ed Clarified border router routing table API description
e4630a4 Wi-SUN interface now informs address changes as interface events
2174374 Fix error found by coverity (ARMmbed#2389)
843254a MPL: traces for transmit and receive message (ARMmbed#2387)

git-subtree-dir: features/nanostack/sal-stack-nanostack
git-subtree-split: 48609ae
artokin pushed a commit to artokin/mbed-os that referenced this issue Aug 18, 2020
…..48609ae

48609ae Merge branch 'release_internal' into release_external
62d8586 Ignore ns_monitor when receiving Ack (ARMmbed#2417)
3323f36 Add support for Ethernet RA dns configuration
d8e7d40 Iotthd 4239 (ARMmbed#2414)
b46f3c6 add empty function for ws_stack_info_get
fc97980 Changed RADIUS shared secret length to 16-bit value
f827ffc Added information API to Wi-SUN and border router
8f1f9d5 EDFE error handling update
51bf94e Fix adaptation interface unit tests (ARMmbed#2409)
0860b57 FHSS_WS: Fixed reading unicast remaining slots (ARMmbed#2408)
4d8c03b Border Router RADIUS client basic authentication functionality (ARMmbed#2406)
fbfada9 Adaptation IF: Allocate fragmentation buffer only if needed (ARMmbed#2407)
66f1bff Added storing of PAN version to NVM on BR
89826ce Iotthd 4224 (ARMmbed#2403)
3fc1ae2 BR EUI-64 is now selected for 4WH using PMKID on 4WH Message 1
af8438e Timing tool traces (ARMmbed#2401)
7938795 Fixed new PD data request for check if EDFE exchange is active.
85ab8fd Extented Frame exchange support
86b1f27 Merge pull request ARMmbed#2399 from ARMmbed/IOTTHD-4220
fed69e0 Add missing test method to ws_empty_functions
6b58e26 Add EDFE mode to Socket API setsockopt
1283077 Test API to adjust 6LoWPAN fragmentation MTU size (ARMmbed#2398)
e787874 Init MAC MTU size based on driver MTU size (ARMmbed#2397)
bf8e89e Ignore neighbors using unsupported channel function (ARMmbed#2395)
1c263fd FHSS exclude channel usage from mask and Brazilian Domain support
841dcbe MAC: Configurable data whitening (ARMmbed#2393)
9a10d66 Fix global address detection (ARMmbed#2392)
f27fe86 Corrected network name and PAN ID change on auth start
bcce0ed Clarified border router routing table API description
e4630a4 Wi-SUN interface now informs address changes as interface events
2174374 Fix error found by coverity (ARMmbed#2389)
843254a MPL: traces for transmit and receive message (ARMmbed#2387)

git-subtree-dir: features/nanostack/sal-stack-nanostack
git-subtree-split: 48609ae
artokin pushed a commit to artokin/mbed-os that referenced this issue Aug 21, 2020
…3fe574..48609ae

48609ae Merge branch 'release_internal' into release_external
62d8586 Ignore ns_monitor when receiving Ack (ARMmbed#2417)
3323f36 Add support for Ethernet RA dns configuration
d8e7d40 Iotthd 4239 (ARMmbed#2414)
b46f3c6 add empty function for ws_stack_info_get
fc97980 Changed RADIUS shared secret length to 16-bit value
f827ffc Added information API to Wi-SUN and border router
8f1f9d5 EDFE error handling update
51bf94e Fix adaptation interface unit tests (ARMmbed#2409)
0860b57 FHSS_WS: Fixed reading unicast remaining slots (ARMmbed#2408)
4d8c03b Border Router RADIUS client basic authentication functionality (ARMmbed#2406)
fbfada9 Adaptation IF: Allocate fragmentation buffer only if needed (ARMmbed#2407)
66f1bff Added storing of PAN version to NVM on BR
89826ce Iotthd 4224 (ARMmbed#2403)
3fc1ae2 BR EUI-64 is now selected for 4WH using PMKID on 4WH Message 1
af8438e Timing tool traces (ARMmbed#2401)
7938795 Fixed new PD data request for check if EDFE exchange is active.
85ab8fd Extented Frame exchange support
86b1f27 Merge pull request ARMmbed#2399 from ARMmbed/IOTTHD-4220
fed69e0 Add missing test method to ws_empty_functions
6b58e26 Add EDFE mode to Socket API setsockopt
1283077 Test API to adjust 6LoWPAN fragmentation MTU size (ARMmbed#2398)
e787874 Init MAC MTU size based on driver MTU size (ARMmbed#2397)
bf8e89e Ignore neighbors using unsupported channel function (ARMmbed#2395)
1c263fd FHSS exclude channel usage from mask and Brazilian Domain support
841dcbe MAC: Configurable data whitening (ARMmbed#2393)
9a10d66 Fix global address detection (ARMmbed#2392)
f27fe86 Corrected network name and PAN ID change on auth start
bcce0ed Clarified border router routing table API description
e4630a4 Wi-SUN interface now informs address changes as interface events
2174374 Fix error found by coverity (ARMmbed#2389)
843254a MPL: traces for transmit and receive message (ARMmbed#2387)

git-subtree-dir: connectivity/nanostack/sal-stack-nanostack
git-subtree-split: 48609ae
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants