-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[Exporter/compile tests] Examples test filters #3206
Conversation
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.
I think there's one little issue, let me know if you think the same.
@@ -198,7 +201,7 @@ def print_message(message, name): | |||
# list of valid combinations to work through | |||
for target, ide in target_cross_ide(ides, | |||
example['features'], | |||
example['targets']): | |||
example['targets'] or targets): |
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.
I'm pretty sure this needs to be a set intersection instead of an OR since both are valid filters.
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.
I thought about it, but basically:
example['targets'] can be the empty set, so there would be no intersection :(
@@ -271,7 +276,8 @@ def compile_repos(config, toolchains): | |||
# Check that the target, toolchain and features combinations are valid and return a | |||
# list of valid combinations to work through | |||
for target, toolchain in target_cross_toolchain(toolchains, | |||
example['features'], example['targets']): | |||
example['features'], | |||
example['targets'] or targets): |
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.
Same thang here, set intersection please!
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.
Caught one last thing
@@ -293,7 +305,7 @@ def compile_repos(config, toolchains): | |||
return results | |||
|
|||
|
|||
def update_mbedos_version(config, tag): | |||
def update_mbedos_version(config, tag, example): |
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.
You've added example
here but you're not using it at all. Can you add the continue
step in the loop here as well? The do_versionning
function will need to be updated in examples.py
as well.
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.
One last thing (for real this time)!
@@ -305,6 +317,8 @@ def update_mbedos_version(config, tag): | |||
""" | |||
print("Updating mbed-os in examples to version %s\n" % tag) | |||
for example in config['examples']: | |||
if examples['name'] not in examples: |
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.
Lil typo here: examples['name']
should be example['name']
1d85a2b
to
dc8a9bb
Compare
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.
Hooray! Looks good to me!
@sarahmarshy can you resolve the conflicts Any testing needed? |
dc8a9bb
to
4bfd73a
Compare
@sg- We'll run it through PR testing to make sure the changes don't affect example building. /morph test |
Result: ABORTEDYour command has finished executing! Here's what you wrote!
Outputmbed Build Number: 1019 Build failed! |
I found an issue with some of the filters in this PR, I went ahead and canceled testing on this PR to allow some other ones through and to get this restarted. /morph test |
There seems to be some other deeper issues with the filtering that I'm able to reproduce. I need to investigate this tomorrow even if the CI says it's ok before this is merged. |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
Outputmbed Build Number: 1022 All builds and test passed! |
2173069
to
2fe1373
Compare
/morph test |
Result: SUCCESSYour command has finished executing! Here's what you wrote!
Outputmbed Build Number: 1027 All builds and test passed! |
Description
Allow command line filtering of targets and examples for both compile and export tests.
Status
READY
Todos