Icetea hw restriction #8137

OPpuolitaival · 2018-09-14T12:48:03Z

Description

Restrict the board where test are running. This fix the problem in case that user has multiple different boards connected to the machine same time

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Breaking change

OPpuolitaival · 2018-09-14T12:48:22Z

@jonikula @studavekar please review

jonikula · 2018-09-14T12:51:26Z

tools/run_icetea.py

 def get_applications(test):
    ret = list()
    for dut in test['requirements']['duts'].values():
-        if 'application' in dut.keys() and 'name' in dut['application'].keys():
+        if 'application' in dut.keys() and 'name' in dut['applicati on'].keys():


thanks, fixed

…time

cmonr · 2018-09-14T14:06:21Z

@jonikula Please take another look as Olli has updated this PR.

OPpuolitaival · 2018-09-14T14:09:41Z

@alekla01 please review

0xc0170 · 2018-09-17T07:54:43Z

/morph build

alekla01

(example, does not necessarily have anything to do with this pr)
target = 'K64F', ['1']['allowed_platforms'] = ['K66F']
doesn't this use 'K64F' binaries but flash 'K66F'?

jupe · 2018-09-17T08:29:01Z

For me it looks like setting platform_name would be the correct way to do this. This now overwrites allowed_platforms list from test case.. Unless there is something I just don't get yet..

mbed-ci · 2018-09-17T08:54:47Z

Build : SUCCESS

Build number : 3081
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/8137/

Triggering tests

/morph test
/morph export-build
/morph mbed2-build

0xc0170 · 2018-09-17T09:02:05Z

@OPpuolitaival Please review the latest 2 comments, if this requires changes . Should be done asap if that is the case

OPpuolitaival · 2018-09-17T09:15:38Z

@jupe Current implementation work surely. You need to understand that it is about generated configuration and we need to limit target devices. Current idea is also that there should not be board specific test cases in Mbed OS. Now it works so that test application binary cannot be build if target board do not support those features. When application cannot be build the tests will be skipped.

OPpuolitaival · 2018-09-17T09:16:32Z

@alekla01 do you mean that tests are not real use cases? Yes those are just unit tests which validate function not the bigger use case

alekla01 · 2018-09-17T09:52:32Z

@OPpuolitaival
No, i mean if you set ['*']['allowed_platforms'] = target, and the test requirements sets ['1']['allowed_platforms'] = [target, target2], target2 may still get flashed with target binary (if i am not mistaken). However, this is not an issue with the current tests and should not be and issue as long as 'allowed_platforms' is only used in ['*'] and not in ['1...10']. ('Current idea is also that there should not be board specific test cases in Mbed OS' -> 'allowed_platforms' is not used in ['1..10'] -> ok). Both allowed_platforms and platform_name work just as well in this case.

mbed-ci · 2018-09-17T10:41:05Z

Test : SUCCESS

Build number : 2870
Test logs :http://mbed-os-logs.s3-website-us-west-1.amazonaws.com/?prefix=logs/8137/2870

jupe · 2018-09-17T11:40:58Z

Current idea is also that there should not be board specific test cases in Mbed OS

In that case this should overwrite allowed list with empty array (="no board specific test") and use platform_name -property to select platform under test explicitly.. Current implementation causes a bit confusion way how it uses allowed_platform -property even it "seems to work as expected" - it works just because icetea picks first platform from that list by default in case of user does not select explicitly platform_name (which was originally implemented kind of backward compatible reason)... Anyway, I'm fine with current approach - it's not perfect but it do it's job for now..

Another concern related to @alekla01 comment. Do we have/will have tests which require different applications per device ? That might causing more tuning here..

mbed-ci · 2018-09-17T12:57:52Z

Exporter Build : SUCCESS

Build number : 2692
Build artifacts/logs : http://mbed-os.s3-website-eu-west-1.amazonaws.com/?prefix=builds/exporter/8137/

0xc0170 · 2018-09-17T14:55:48Z

@jupe @alekla01 @OPpuolitaival All approved but based on the comments, the discussion is not yet over. I believe this is good to go (needs some improvements later possibly). If not, now it's the time to request changes.

cmonr · 2018-09-17T15:06:51Z

Bringing this in, since we need to get RC3 generated in the next couple of hours. Further discussion can continue either here or in an issue (prefereably an issue).

OPpuolitaival · 2018-09-17T17:46:31Z

@0xc0170 this solve the problem. Yes there might be some issues in future that we might need to change this a bit. The reason why this code is in this repository and not in mbed-cli is that icetea usage logic and test cases will be in sync

Olli-Pekka Puolitaival added 2 commits September 14, 2018 13:49

Include run_icetea unittests also in pytest run

cd628a7

Fix tests

1981728

jonikula suggested changes Sep 14, 2018

View reviewed changes

Fix icetea run in case that user has different boards connected same …

ed18665

…time

OPpuolitaival force-pushed the icetea_hw_restriction branch from f6aaff4 to ed18665 Compare September 14, 2018 13:29

cmonr added needs: review release-version: 5.10.0-rc3 labels Sep 14, 2018

cmonr requested a review from alekla01 September 14, 2018 14:12

OPpuolitaival force-pushed the icetea_hw_restriction branch from c648019 to ed18665 Compare September 14, 2018 14:48

cmonr added the risk: A label Sep 14, 2018

jonikula approved these changes Sep 17, 2018

View reviewed changes

0xc0170 approved these changes Sep 17, 2018

View reviewed changes

alekla01 reviewed Sep 17, 2018

View reviewed changes

alekla01 approved these changes Sep 17, 2018

View reviewed changes

cmonr added needs: CI needs: review and removed needs: review labels Sep 17, 2018

cmonr added ready for merge and removed needs: CI labels Sep 17, 2018

cmonr merged commit 7a0c9a6 into ARMmbed:master Sep 17, 2018

0xc0170 removed needs: review ready for merge labels Sep 17, 2018

Icetea hw restriction #8137

Icetea hw restriction #8137

Uh oh!

Conversation

OPpuolitaival commented Sep 14, 2018

Description

Pull request type

Uh oh!

OPpuolitaival commented Sep 14, 2018

Uh oh!

jonikula Sep 14, 2018

Choose a reason for hiding this comment

Uh oh!

OPpuolitaival Sep 14, 2018

Choose a reason for hiding this comment

Uh oh!

cmonr commented Sep 14, 2018

Uh oh!

OPpuolitaival commented Sep 14, 2018

Uh oh!

0xc0170 commented Sep 17, 2018

Uh oh!

alekla01 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jupe commented Sep 17, 2018

Uh oh!

mbed-ci commented Sep 17, 2018

Build : SUCCESS

Triggering tests

Uh oh!

0xc0170 commented Sep 17, 2018

Uh oh!

OPpuolitaival commented Sep 17, 2018

Uh oh!

OPpuolitaival commented Sep 17, 2018

Uh oh!

alekla01 commented Sep 17, 2018

Uh oh!

mbed-ci commented Sep 17, 2018

Test : SUCCESS

Uh oh!

jupe commented Sep 17, 2018

Uh oh!

mbed-ci commented Sep 17, 2018

Exporter Build : SUCCESS

Uh oh!

0xc0170 commented Sep 17, 2018

Uh oh!

cmonr commented Sep 17, 2018

Uh oh!

OPpuolitaival commented Sep 17, 2018

Uh oh!

Uh oh!

alekla01 left a comment •

edited

Loading