The skipping of items without a relevant path was failing to pick ite… #26328
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ms with the final element set to None. Now we skip whenever the final element in the path is None.
|
|
|
PR #26328: Size comparison from fa505f6 to f2b9ce8 Full report (4 builds for cc32xx, mbed, qpg)
|
|
PR #26328: Size comparison from f587c34 to 0c53e1f Decreases (1 build for cc32xx)
Full report (1 build for cc32xx)
|
| if len(path) == 1 and path[0] is None: | ||
| if path[-1] is None: |
There was a problem hiding this comment.
What was the actual tuple for the failing path here? How did we get something where the "leaf" is None but the length is not 1?
There was a problem hiding this comment.
That is a fair point. TBH I did not put in a lot of investigation time to figure out how the situation arose. I printed the list of paths before the for loop and got
ApplicationPaths(
chip_tool=['ip', 'netns', 'exec', 'tool', './examples/chip-tool/out/debug/chip-tool'],
all_clusters_app=['ip', 'netns', 'exec', 'app', 'examples/all-clusters-app/linux/out/debug/chip-all-clusters-app'],
lock_app=['ip', 'netns', 'exec', 'app', None],
ota_provider_app=['ip', 'netns', 'exec', 'app', None],
ota_requestor_app=['ip', 'netns', 'exec', 'app', None],
tv_app=['ip', 'netns', 'exec', 'app', None],
bridge_app=['ip', 'netns', 'exec', 'app', None],
chip_repl_yaml_tester_cmd=['ip', 'netns', 'exec', 'tool', 'python3', '/home/whicklin/Documents/openRepos/connectedhomeip/scripts/tests/chiptest/yamltest_with_chip_repl_tester.py'],
chip_tool_with_python_cmd=['ip', 'netns', 'exec', 'tool', 'python3', '/home/whicklin/Documents/openRepos/connectedhomeip/scripts/tests/yaml/chiptool.py']
)
so I thought the path might be being built by appending to a list and that in some cases it was appending none.
There was a problem hiding this comment.
Ah, on Linux there are these netns prefixes on everything, I see. Thank you!
There was a problem hiding this comment.
I wonder if we should not instead here make netns not create a list and have None preserved if the input path is None.
Even more, we should probably have None or List of valid strings rather than trying to accept [None] and detect it. We seem to introduce odd data along the pipeline and then try to deal with it. I would rather not have odd data to begin with.
There was a problem hiding this comment.
Doable, but requires finding all places that manipulate this path and fixing them....
pullapprove
bot
requested review from
amitnj,
andy31415,
anush-apple,
arkq,
carol-apple,
chrisdecenzo,
chshu,
chulspro,
cliffamzn,
CodeChronos928,
Damian-Nordic,
dhrishi,
electrocucaracha,
franck-apple,
gjc13,
harimau-qirex,
harsha-rajendran,
hawk248,
jelderton,
jepenven-silabs and
jmartinez-silabs
pullapprove
bot
requested review from
jtung-apple,
kcoppock,
kkasperczyk-no,
ksperling-apple,
lazarkov,
lpbeliveau-silabs,
LuDuda,
mrjerryjohns,
msandstedt,
mspang,
nivi-apple,
robszewczyk,
saurabhst,
selissia,
sharadb-amazon,
tcarmelveilleux,
tecimovic,
turon,
vijs,
woody-apple,
xylophone21,
younghak-hwang and
yunhanw-google
bzbarsky-apple
approved these changes
May 5, 2023
|
Fast-tracking: this has been up for review for a while, and is a tests-only fix. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Upon running
./scripts/tests/run_test_suite.py --chip-tool ./examples/chip-tool/out/debug/chip-tool --target 'TestModeSelectCluster' run --all-clusters-app examples/all-clusters-app/linux/out/debug/chip-all-clusters-appto run the tests against an application, the following error was returned.This is caused by the
test_definition.pyscript not skipping invalid application paths because those paths happen to have a length != 1. This PR skips all paths where the last element isNone, meaning that it is invalid.