Skip to content

Travis: Tear out python3 testing to get CI working again #6218

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

Merged
merged 3 commits into from
Feb 27, 2018

Conversation

geky
Copy link
Contributor

@geky geky commented Feb 26, 2018

Temporary solution, python3 will be added back when working (see #6200)

Travis doesn't recognize version matrices when inside a matrix include job. Instead, it breaks in a way that isn't reported as an error.

cc @theotherjimmy, @0xc0170, @deepikabhavnani, @cmonr
intended for patch
introduced in #5848
related travis-ci/travis-ci#9275

Temporary solution, python3 will be added back later

Travis doesn't recognize version matrices when inside a matrix include job.
Instead, it breaks in a way that isn't reported as an error.
theotherjimmy
theotherjimmy previously approved these changes Feb 26, 2018
Copy link
Contributor

@theotherjimmy theotherjimmy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Much better than doing nothing silently.

@cmonr
Copy link
Contributor

cmonr commented Feb 26, 2018

/morph build

cmonr
cmonr previously approved these changes Feb 26, 2018
@geky geky mentioned this pull request Feb 26, 2018
6 tasks
@mbed-ci
Copy link

mbed-ci commented Feb 26, 2018

Build : SUCCESS

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

Triggering tests

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

@geky
Copy link
Contributor Author

geky commented Feb 26, 2018

@cmonr, sorry it's not ready yet. Now, travis fails successfully, but we need to fix the tools so it does not.

@cmonr
Copy link
Contributor

cmonr commented Feb 26, 2018

@geky Oh, my mistake. Was under the impression that the quick fix was ready. In that case, LMK when it is.

@cmonr cmonr dismissed their stale review February 26, 2018 20:31

PR not ready yet.

@mbed-ci
Copy link

mbed-ci commented Feb 26, 2018

@geky
Copy link
Contributor Author

geky commented Feb 26, 2018

Bugs introduced while broken:

@theotherjimmy
Copy link
Contributor

#6096 did not have to add the offending line; it added nothing.

@theotherjimmy
Copy link
Contributor

The first error is a unicode problem XD notice the u'...'

@mbed-ci
Copy link

mbed-ci commented Feb 26, 2018

@deepikabhavnani
Copy link

#6096 did not have to add the offending line; it added nothing.

@theotherjimmy - offending line was a suggestion from your side to get the path of build directory, which is used few steps below.
secure_file = join(build_dir, "cmse_lib.o")

Will appreciate to have proper fix from your side, to append the build directory path for cmse_lib.o creation.

@geky
Copy link
Contributor Author

geky commented Feb 26, 2018

I added some fixes/workarounds, @theotherjimmy, @deepikabhavnani, if you could review and let me know if you're happy with these for now.

The first error is a unicode problem XD notice the u'...'

Haha, it's not even a real unicode error. It's just that now the config will show that "u" when it has errors, and the tests are checking the error strings.

offending line was a suggestion from your side to get the path of build directory, which is used few steps below.

@deepikabhavnani, I moved the kwargs['build_dir'] into the condition for "secure". This lets the test pass for now, let me know if you're happy with this as a workaround so we can get tool testing back into mbed OS.

@theotherjimmy
Copy link
Contributor

@geky That workaround works, but I would prefer that the offending test just pass in a dummy build_dir parameter. @deepikabhavnani My bad, I thought that line was in the ARM_STD constructor. It makes sense where it is.

@deepikabhavnani
Copy link

deepikabhavnani commented Feb 26, 2018

I moved the kwargs['build_dir'] into the condition for "secure". This lets the test pass for now, let me know if you're happy with this as a workaround so we can get tool testing back into mbed OS.

@theotherjimmy - Can you please confirm build_dir will not be none for Cortex-M23/M33 devices.

Toolchain init is different for ARM / ARMC6, will this have any impact?
https://github.com/geky/mbed/blob/962be07a31586f6cc8d8e2ae8dddb7b05b7e4554/tools/toolchains/arm.py#L49
https://github.com/geky/mbed/blob/962be07a31586f6cc8d8e2ae8dddb7b05b7e4554/tools/toolchains/arm.py#L307

Ok, i was late :-)

@theotherjimmy
Copy link
Contributor

@deepikabhavnani Build dir being None is an error. We need a place to put object files.

@theotherjimmy
Copy link
Contributor

@geky Feel free to delete the test_instantiation test. It adds no value anymore.

@theotherjimmy
Copy link
Contributor

It added value at one point, but no longer.

@geky geky force-pushed the g-fix-travis-python-versions-2 branch from 962be07 to f88536c Compare February 26, 2018 23:22
@geky
Copy link
Contributor Author

geky commented Feb 26, 2018

@geky Feel free to delete the test_instantiation test. It adds no value anymore.

Cool beans, it's gone now.

@cmonr Should be good to go now. Though no rush, Travis is severly backlogged because of mbed 2 builds.

cmonr
cmonr previously approved these changes Feb 27, 2018
@cmonr
Copy link
Contributor

cmonr commented Feb 27, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Feb 27, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci
Copy link

mbed-ci commented Feb 27, 2018

@mbed-ci
Copy link

mbed-ci commented Feb 27, 2018

Also moved the access of build_dir into condition on secure in ARMC6

per theotherjimmy
@geky geky force-pushed the g-fix-travis-python-versions-2 branch from f88536c to 24bb556 Compare February 27, 2018 15:39
@geky
Copy link
Contributor Author

geky commented Feb 27, 2018

@cmonr, sorry for the slip up. Should be good to go now (heavy handed solution).

I moved the build dir back into the condition. It looks like none of the other tests pass in build_dir either, but the root of this problem can be fixed in another pr.

@cmonr
Copy link
Contributor

cmonr commented Feb 27, 2018

/morph build

@mbed-ci
Copy link

mbed-ci commented Feb 27, 2018

Build : SUCCESS

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

Triggering tests

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

@mbed-ci
Copy link

mbed-ci commented Feb 27, 2018

@mbed-ci
Copy link

mbed-ci commented Feb 27, 2018

Test : SUCCESS

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

@cmonr cmonr removed the needs: CI label Feb 27, 2018
@cmonr cmonr merged commit 5077644 into ARMmbed:master Feb 27, 2018
@adbridge
Copy link
Contributor

adbridge commented Mar 14, 2018

Reverting a change that only went in for 5.8 so should have also targeted that release! In fact as this was merged prior to 5.8 rc1 it should already be in that release

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

Successfully merging this pull request may close these issues.

7 participants