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

Make examples commands return a failure #8605

Merged
merged 2 commits into from Nov 5, 2018

Conversation

Projects
None yet
9 participants
@adbridge
Contributor

adbridge commented Oct 31, 2018

Description

Currently the following commands in examples.py,
do_import()
do_deploy()
do_versionning()
do_clone()

all return a success status (ie 0) irrespective of any errors
originating from their sub-functions.

This PR fixes this. Now these commands will return one of:
0 - success
1 - general failure
x - failure returned by a subprocess.call function

Pull request type

[x] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[ ] Test update
[ ] Breaking change
Make examples commands return a failure
Currently the following commands in examples.py,
do_import()
do_deploy()
do_versionning()
do_clone()

all return a success status (ie 0) irrespective of any errors
originating from their sub-functions.

This PR fixes this. Now these commands will return one of:
0 - success
1 - general failure
x - failure returned by a subprocess.call function

@ciarmcom ciarmcom requested review from ARMmbed/mbed-os-maintainers Oct 31, 2018

@ciarmcom

This comment has been minimized.

Member

ciarmcom commented Oct 31, 2018

@adbridge, thank you for your changes. @ARMmbed/mbed-os-tools @ARMmbed/mbed-os-maintainers please review.

@adbridge

This comment has been minimized.

Contributor

adbridge commented Oct 31, 2018

Fixes #8568

@cmonr cmonr requested a review from ARMmbed/mbed-os-test Oct 31, 2018

@cmonr

cmonr approved these changes Oct 31, 2018

@OPpuolitaival

This comment has been minimized.

Contributor

OPpuolitaival commented Oct 31, 2018

@adbridge Did you test this? I tested a bit suprocess.call() function and seems to work in a way that:

  • subprocess.call("exit 1") => not returning anything, just raising an error
  • subprocess.call("exit 1", shell=True) => return a return code

It seems that you expect the behaviour as shell=True but it is false as default

@OPpuolitaival

This comment has been minimized.

Contributor

OPpuolitaival commented Oct 31, 2018

[examples] Running shell script

  • python -u ../mbed-os/tools/test/examples/examples.py clone
    Traceback (most recent call last):
    File "../mbed-os/tools/test/examples/examples.py", line 17, in
    import examples_lib as lib
    File "/builds/ws/ci_PR-8605-QHY26DPOP4ARV3WYD2LBS/mbed-os/tools/test/examples/examples_lib.py", line 421
    return result:
    ^
    SyntaxError: invalid syntax
    script returned exit code 1
@OPpuolitaival

This comment has been minimized.

Contributor

OPpuolitaival commented Oct 31, 2018

Now fixed CI so that it also catch examples fetching problems right way.

@cmonr

This comment has been minimized.

Contributor

cmonr commented Oct 31, 2018

Now fixed CI so that it also catch examples fetching problems right way.

@OPpuolitaival Are you referring to this?
https://github.com/ARMmbed/mbed-os-ci/pull/190/files

os.chdir("../..")
if result:
return result:

This comment has been minimized.

@bridadan

bridadan Oct 31, 2018

Contributor

@OPpuolitaival found this, but the correct thing here would be:

Suggested change Beta
return result:
return result

This comment has been minimized.

@adbridge

adbridge Nov 1, 2018

Contributor

Yup copy paste there me thinks.

@adbridge

This comment has been minimized.

Contributor

adbridge commented Nov 1, 2018

@OPpuolitaival mmmm I just looked at a call that was already in the code:

if subprocess.call([repo_info['type'], "clone", repo_info['repo']]) == 0:

So from what you said that would never have worked either.

@adbridge

This comment has been minimized.

Contributor

adbridge commented Nov 1, 2018

Fixing...

Fix up subprocess calls
subprocess.call() does not by default return a status value.

Update the commands to add shell=True which forces a return value.
Also convert the commands to a single string rather than a list as
this plays more nicely with both linux and windows.

Also fix a spurious :
@NirSonnenschein

This comment has been minimized.

Contributor

NirSonnenschein commented Nov 4, 2018

/morph build

@mbed-ci

This comment has been minimized.

mbed-ci commented Nov 4, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci

This comment has been minimized.

@mbed-ci

This comment has been minimized.

@0xc0170 0xc0170 merged commit 909c11b into ARMmbed:master Nov 5, 2018

15 checks passed

ci-morph-build build completed
Details
ci-morph-exporter build completed
Details
ci-morph-mbed2-build build completed
Details
ci-morph-test test completed , RTOS ROM(+0 bytes) RAM(+0 bytes)
Details
continuous-integration/jenkins/pr-head This commit looks good
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
jenkins-ci/cloud-client-test Success
Details
jenkins-ci/unittests Success
Details
travis-ci/astyle Passed, 545 files (+0 files)
Details
travis-ci/docs Local docs testing has passed
Details
travis-ci/events Passed, runtime is 9274 cycles (-769 cycles)
Details
travis-ci/gitattributestest Local gitattributestest testing has passed
Details
travis-ci/licence_check Local licence_check testing has passed
Details
travis-ci/littlefs Passed, code size is 8372B (+0.00%)
Details
travis-ci/tools-py2.7 Local tools-py2.7 testing has passed
Details

@0xc0170 0xc0170 removed the ready for merge label Nov 5, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment