Skip to content
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

ARM test automation #6150

Closed
wants to merge 11 commits into from
Closed

ARM test automation #6150

wants to merge 11 commits into from

Conversation

yichoi
Copy link
Contributor

@yichoi yichoi commented May 1, 2013

Support #5297

install.mk : install-runtime-target added for conveneice
automatically push runtime library to android device

test.mk : expanded to support android test automation with adb

compiletest : expanded to support android test automation with adb

@brson
Copy link
Contributor

brson commented May 2, 2013

Wow. Looks impressive. I'll try to run this locally.

@graydon
Copy link
Contributor

graydon commented May 2, 2013

This looks good to me, but I think it's @brson's call on whether it needs any other adjustments.

Thanks!

@brson
Copy link
Contributor

brson commented May 2, 2013

This adds a new assumption to compiletest that when host != target then we require special cases to run the tests. This is not true though for x86_64 vs. i686 - we do cross compile from x86_64 to i686 and then run the tests on the host platform. The effect of this patch will be that the i686 tests never get run on the bots. Likewise the tests for CFG_RUNNABLE_* appear to disable the i686 tests on x86_64.

Perhaps instead of using host and target, we can just base the decision to use adb on whether the adb_path is provided, and/or on whether the target is android.

The 'runnable' flag to compiletest is supposed to indicate whether adb is available and, if it isn't, just compile the tests but not run them. I think this may be too clever, resulting in situations where it looks like you've tested your code but actually haven't (e.g. it looks like i686 is being tested but it isn't). Can we possibly make it so that adb is required to test android?

All four of the new compiletest flags seem to be used to decide the question 'should I use adb to run the tests?'. That could be indicated with a single flag instead of four, though admittedly that wouldn't be as general a solution.

@brson
Copy link
Contributor

brson commented May 2, 2013

So far when I try to run this on the emulator I always see check: adb device not attached

@brson
Copy link
Contributor

brson commented May 2, 2013

When I run this the tests fail with errors like

rust: task failed at 'explicit failure', /home/brian/dev/rust/src/compiletest/runtest.rs:813
test [run-pass] /home/brian/dev/rust/src/test/run-pass/attr-main-2.rs ... FAILED
------stdout------------------------------
/system/bin/sh: cd: /system/tmp: No such file or directory

------stderr------------------------------
/system/bin/sh: cd: /system/tmp: No such file or directory
/system/bin/sh: ./test-runner-hides-main.stage1-arm-linux-androideabi: not found

@brson
Copy link
Contributor

brson commented May 2, 2013

The above was because my emulator has a read-only filesystem, so /system/tmp wasn't created.

@yichoi
Copy link
Contributor Author

yichoi commented May 3, 2013

if device is not rooted or one wants to change android test dir,

one can change like below
install.mk:222 CFG_RUNTIME_PUSH_DIR=/data/tmp
test.mk:815 CFG_ADB_TEST_DIR=/data/tmp

then make install-target-runtime, make check-something

In case of android emulator, libgnustl_shared.so is missing.
one should manually push like "adb push libgnustl_shared.so /data/tmp"

libgnustl_shared.so can be found from android ndk (e.g. ~ndk_standalone/arm-linux-androideabi/lib/armv7-a )

@yichoi
Copy link
Contributor Author

yichoi commented May 3, 2013

Basically host != target indicate that target cannot run on this host. I think x86_64 vs. i686 is special case can run with the help of supported library. one can handle this special case by expanding match cases with (target, flagrunnable).

I will modify apparent 3 things as you suggested

  • configure : find adb, make : check adb device attached
  • make : check runnable more precisely (e.g. test dir writable)
  • simplify redundant parts to add adb-path

@yichoi
Copy link
Contributor Author

yichoi commented May 3, 2013

configure : CFG_ADB added for adb path
install.mk

  • make install-runtime-target : push to default path of target (/system/lib, rooted device)
  • make install-runtime-target /data/tmp : push to specific path of target

test.mk and compiletest

  • regardless of host and target, compiletest like previously but,
  • in case of arm-linux-androideabi and device attached, special preparation added
  • in case of arm-linux-androideabi and device detached, skip run test.

@yichoi
Copy link
Contributor Author

yichoi commented May 4, 2013

Code Cleanup and looks to me almost completed

One can make ARM automation like hudson with below instruction

  • run emulator : [android SDK path]/tools/emulator64-arm -avd [profile name] -no-window
    (It could be east to make avd profile with Android Virtual Device Manager)
  • push runtime : make install-runtime-target (rooted)
  • push runtime : make install-runtime-target /data/tmp (non-rooted)
  • do test : make check_something_

While ARM testing, I encountered several assertion of memory_region destructor. :(

@brson
Copy link
Contributor

brson commented May 6, 2013

Thanks for the followup commits @yichoi

bors added a commit that referenced this pull request May 6, 2013
Support #5297

install.mk : install-runtime-target added for conveneice
                 automatically push runtime library to android device

test.mk : expanded to support android test automation with adb

compiletest : expanded to support android test automation with adb
@bors bors closed this May 6, 2013
bors added a commit to rust-lang-ci/rust that referenced this pull request Oct 24, 2020
Update backport documentation to the subtree workflow

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

Successfully merging this pull request may close these issues.

4 participants