Consider merging arcapp package to arc package |
|||
Issue descriptionMy initial intention to introduce arcapp package was to separate ARC tests and ARC application tests; the former is to test the core functionalities of Android on Chrome OS, and the latter is to test app compatibilities of ARC. However recently people are adding ARC core tests to arcapp package because arcapp/apptest package is (very slightly) useful for writing tests involving UI automator, which I did not expect initially. I never intend to blame test writers, but I'm blaming myself; I now think introduction of arcapp/apptest was a premature optimization. So I'm considering the following change: - All arcapp.* tests will be renamed to arc.* - arcapp.Sample will be arc.UIAutomator - apptest package will be removed and inlined to individual tests.
,
Dec 18
+people making/made changes to arcapp package
,
Dec 18
Thanks for cleaning this up! Is it worthwhile moving the code from apptest.go to some other location? If we have many tests calling functions in exactly the same order with the same arguments, I'm probably supportive of adding a helper function.
,
Dec 18
For now, please let me delete apptest. After we get more ARC tests, we'll see what kind of helper function is best for use cases.
,
Dec 19
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/platform/tast-tests/+/6d856f2e385c26ffce9c2f98175e7747425faa2c commit 6d856f2e385c26ffce9c2f98175e7747425faa2c Author: Shuhei Takahashi <nya@chromium.org> Date: Wed Dec 19 09:12:49 2018 arc: Remove arcapp/apptest packages. My initial intention to introduce arcapp package was to have ARC core tests in arc package and ARC application compatibility tests in arcapp package. apptest package was introduced hoping to make it easier to write application tests since most tests were expected to use the same boilerplate code to set up UI automator. Today we have no application compatibility tests yet (except for arcapp.Sample), but people place ARC core functionality tests to arcapp package just to use apptest. I now think it was premature optimization to introduce arcapp/apptest. This change moves existing tests in arcapp package to arc package, and inlines apptest calls to individual tests. BUG= chromium:916060 TEST=tast run DUT '(!disabled && "name:arc.*")' Change-Id: Id0574059e7f70866ea3cfcdc063ccf5c2a1b2455 Reviewed-on: https://chromium-review.googlesource.com/1382191 Commit-Ready: Shuhei Takahashi <nya@chromium.org> Tested-by: Shuhei Takahashi <nya@chromium.org> Reviewed-by: Dan Erat <derat@chromium.org> Reviewed-by: Hidehiko Abe <hidehiko@chromium.org> [add] https://crrev.com/6d856f2e385c26ffce9c2f98175e7747425faa2c/src/chromiumos/tast/local/bundles/cros/arc/data/todo-mvp.apk.external [rename] https://crrev.com/6d856f2e385c26ffce9c2f98175e7747425faa2c/src/chromiumos/tast/local/bundles/cros/arc/media_session_gain_transient.go [rename] https://crrev.com/6d856f2e385c26ffce9c2f98175e7747425faa2c/src/chromiumos/tast/local/bundles/cros/arc/mediasession/mediasession.go [rename] https://crrev.com/6d856f2e385c26ffce9c2f98175e7747425faa2c/src/chromiumos/tast/local/bundles/cros/arc/media_session_gain.go [rename] https://crrev.com/6d856f2e385c26ffce9c2f98175e7747425faa2c/src/chromiumos/tast/local/bundles/cros/arc/media_session_gain_transient_duck.go [add] https://crrev.com/6d856f2e385c26ffce9c2f98175e7747425faa2c/src/chromiumos/tast/local/bundles/cros/arc/data/media_session_60sec_test.ogg.external [delete] https://crrev.com/2fe995f1f7159bbb2a007aadf5369d926d24ff33/src/chromiumos/tast/local/bundles/cros/arcapp/sample.go [delete] https://crrev.com/2fe995f1f7159bbb2a007aadf5369d926d24ff33/src/chromiumos/tast/local/bundles/cros/arcapp/data/media_session_test.apk.external [delete] https://crrev.com/2fe995f1f7159bbb2a007aadf5369d926d24ff33/src/chromiumos/tast/local/bundles/cros/arcapp/apptest/apptest.go [delete] https://crrev.com/2fe995f1f7159bbb2a007aadf5369d926d24ff33/src/chromiumos/tast/local/bundles/cros/arcapp/data/todo-mvp.apk.external [add] https://crrev.com/6d856f2e385c26ffce9c2f98175e7747425faa2c/src/chromiumos/tast/local/bundles/cros/arc/data/media_session_test.apk.external [modify] https://crrev.com/6d856f2e385c26ffce9c2f98175e7747425faa2c/src/chromiumos/tast/local/bundles/cros/main.go [add] https://crrev.com/6d856f2e385c26ffce9c2f98175e7747425faa2c/src/chromiumos/tast/local/bundles/cros/arc/ui_automator.go [delete] https://crrev.com/2fe995f1f7159bbb2a007aadf5369d926d24ff33/src/chromiumos/tast/local/bundles/cros/arcapp/timeout_test.go [delete] https://crrev.com/2fe995f1f7159bbb2a007aadf5369d926d24ff33/src/chromiumos/tast/local/bundles/cros/arcapp/data/media_session_60sec_test.ogg.external [rename] https://crrev.com/6d856f2e385c26ffce9c2f98175e7747425faa2c/src/chromiumos/tast/local/bundles/cros/arc/data/media_session_test.html
,
Jan 7
|
|||
►
Sign in to add a comment |
|||
Comment 1 by nya@chromium.org
, Dec 18