autotest: Refactor policy proto generation |
||||||||
Issue descriptionRight now, every autotest that uses policy code compiles its own policy protos and passes its srcdir to the policy code so that it can import the protos. There's a lot of duplicate code (Makefiles) and it's not very efficient. Instead, there should be one ebuild that builds policy protos and policy autotests should reference it.
,
Dec 14 2017
Issue 773006 has been merged into this issue.
,
Dec 14 2017
,
Jan 22 2018
,
Jan 22 2018
,
Jan 22 2018
,
Mar 13 2018
Moving the discussion from CL:817437 to this bug for better visibility and as reference for my Future Self. TL;DR: It looks like CL:817437 doesn't get applied in the autotest artifacts. I've been trying to submit CL:817436 (ebuild for policy protos) and CL:817437 (refactoring tests using policy) for a while now. Tryjobs and presubmit succeeds, but submit fails. Tryjobs I tested and succeeded include cros tryjob -g 817437 -g 817436 --hwtest chell-full cros tryjob -g 817437 -g 817436 --hwtest betty-paladin-tryjob cros tryjob -g 817437 -g 817436 --hwtest elm-paladin-tryjob cros tryjob -g 817437 -g 817436 --hwtest wolf-paladin-tryjob cros tryjob -g 817437 -g 817436 --hwtest nyan_big-paladin-tryjob Eric mentioned chromium:716151 which seems to be hitting a lot of autotest CLs lately. The test mentioned that changes from CLs don't make it to the autotest build artifacts. Thus, I did some forensics on one of the bots that failed to submit: https://logs.chromium.org/v/?s=chromeos%2Fbb%2Fchromeos%2Fveyron_mighty-paladin%2F8133%2F%2B%2Frecipes%2Fsteps%2FHWTest__bvt-inline_%2F0%2Fstdout I downloaded the corresponding autotest_packages.tar file from https://pantheon.corp.google.com/storage/browser/chromeos-image-archive/veyron_mighty-paladin/R66-10447.0.0-rc1?pli=1 First I checked whether the dep file that contains the policy protos is there and voila, it's there: autotest_packages.tar/packages/dep-policy_protos.tar.bz2 and it contains the expected compile proto pb2.py files. This means, CL:817436 gets applied. Next, I checked one of the failing tests, login_RemoteOwnership. autotest_packages.tar/packages/dep-policy_protos.tar.bz2 This file still contains src/Makefile and login_RemoteOwnership.py which calls utils.make('OUT_DIR=.'). However, 817437 removes the Makefile and calls policy.install_profobufs instead: https://chromium-review.googlesource.com/c/chromiumos/third_party/autotest/+/817437/14/client/site_tests/login_RemoteOwnership/login_RemoteOwnership.py This means, CL:817437 didn't get applied.
,
Mar 13 2018
... or maybe the autotest-tests-ownershipapi ebuild that contains login_RemoteOwnership wasn't rebuilt? From build logs: [ebuild U ] chromeos-base/autotest-tests-ownershipapi-0.0.1-r7875::chromiumos [0.0.1-r7874::chromiumos] to /build/veyron_mighty/ USE="autotest buildcheck opengles tpmtools xset -cros_host -profiling (-#%) (-Tests%) (-Uses%) (-chrome%) (-depend%) (-dependency.%) (-on%) (-telemetry.%) (-test%) (-that%)" TESTS="login_CryptohomeOwnerQuery login_GuestAndActualSession login_MultiUserPolicy login_MultipleSessions login_OwnershipApi login_OwnershipNotRetaken login_OwnershipRetaken login_OwnershipTaken login_RemoteOwnership login_UserPolicyKeys (-login_OwnershipTakenTelemetry%*)" 0 KiB Fetched chromeos-base/autotest-tests-ownershipapi-0.0.1-r7875 in 0.13s Started chromeos-base/autotest-tests-ownershipapi-0.0.1-r7875 (logged in /tmp/autotest-tests-ownershipapi-0.0.1-r7875-2tc5R1) Completed chromeos-base/autotest-tests-ownershipapi-0.0.1-r7875 (in 1m49.0s) 0.0.1-r7874 is the old version. Looks like it got uprev'ed, but the uprev didn't pick up my CL?
,
Mar 14 2018
yet another occurrence of Issue 782034 ?
,
Mar 14 2018
Looks like C:817437 does get applied: https://logs.chromium.org/v/?s=chromeos%2Fbb%2Fchromeos%2Fveyron_mighty-paladin%2F8133%2F%2B%2F%2A%2A%2Fstdout&s=chromeos%2Fbb%2Fchromeos%2Fveyron_mighty-paladin%2F8133%2F%2B%2F%2A%2A%2Fstderr 13:05:44: INFO: Attempting to apply change chromiumos/third_party/autotest:refs/changes/37/817437/14:e636dab1 "autotest: Refactor policy proto code" 13:05:45: INFO: Applying via cherry-pick. Trying simple merge. Simple merge failed, trying Automatic merge. Auto-merging client/site_tests/login_GuestAndActualSession/login_GuestAndActualSession.py ... delete mode 100644 client/site_tests/login_RemoteOwnership/src/Makefile This deletes the Makefile that nevertheless ends up in autotest_packages.tar/packages/dep-policy_protos.tar.bz2. I'm wondering whether the packages get created before the tests are installed, so the packager uses stale files. They're installed by autotest-tests-ownershipapi-9999.ebuild's src_compile() and packaged up in autotest-9999.ebuild's pkg_postinst(). Here's my theory: - autotest_src_compile() creates packages with the 'tar_only' option and it picks up EVERY test that is in client/tests and client/site_tests. It gets called in a couple of random places. - autotest-9999.ebuild's pkg_postinst() uploads packages with the 'upload' option and --client, which uploads all tests, but it only recreates the tar if it doesn't exist yet. - Therefore, tar_only might tar up stale tests and upload won't fix it. Possible solution: Remove if os.path.exists(tarball_path): in packages.py.
,
Mar 17 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/e532d70861c4a5e3274c0c4b8757e832796208ce commit e532d70861c4a5e3274c0c4b8757e832796208ce Author: Aviv Keshet <akeshet@chromium.org> Date: Sat Mar 17 03:55:40 2018 Revert "Reland recent autotest.eclass packaging-related changes" This reverts commit 297bd795ab01b13bce81897376e4b2b4ee7bec63. The lazy packaging is still apparently causing issues, frequently, in the CQ. BUG=chromium:716151, chromium:793333 , chromium:782034 TEST=None Change-Id: I72fc2c9149ed5d89b4b5dc91d80aa9536a985e57 Reviewed-on: https://chromium-review.googlesource.com/961666 Commit-Ready: Aviv Keshet <akeshet@chromium.org> Tested-by: Aviv Keshet <akeshet@chromium.org> Reviewed-by: Lutz Justen <ljusten@chromium.org> Reviewed-by: Allen Li <ayatane@chromium.org> [modify] https://crrev.com/e532d70861c4a5e3274c0c4b8757e832796208ce/eclass/autotest.eclass
,
Mar 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/overlays/chromiumos-overlay/+/488cf8224808f4a0426120e2c65e2042be5a4875 commit 488cf8224808f4a0426120e2c65e2042be5a4875 Author: Lutz Justen <ljusten@chromium.org> Date: Sun Mar 18 00:22:34 2018 autotest: Add ebuild to build Python policy protos Right now, every autotest that uses policy code compiles its own policy protos. There are over 30 nearly identical Makefiles for this. This CL introduces an autotest-deps ebuild that generates the protos. The files are used by all policy autotests in the dependent CL, so there's no code duplication anymore. CQ-DEPEND=CL:817437 BUG= chromium:793333 TEST=test_that -b ${BOARD} ${DUT_IP} login_OwnershipTaken login_OwnershipApi login_OwnershipRetaken TEST=test_that -b ${BOARD} ${DUT_IP} policy_ImagesAllowedForUrls --args='case=3Urls_Allow' TEST=test_that -b ${BOARD} ${DUT_IP} suite:policy suite:bvt-perbuild TEST=cros tryjob -g 817437 -g 817436 --hwtest chell-full Change-Id: I254dcbd3107ded523d3aef3e2b3429d3acbe9f84 Reviewed-on: https://chromium-review.googlesource.com/817436 Commit-Ready: Lutz Justen <ljusten@chromium.org> Tested-by: Lutz Justen <ljusten@chromium.org> Reviewed-by: Lutz Justen <ljusten@chromium.org> [modify] https://crrev.com/488cf8224808f4a0426120e2c65e2042be5a4875/chromeos-base/autotest-tests-ownershipapi/autotest-tests-ownershipapi-9999.ebuild [modify] https://crrev.com/488cf8224808f4a0426120e2c65e2042be5a4875/chromeos-base/autotest-tests/autotest-tests-9999.ebuild [modify] https://crrev.com/488cf8224808f4a0426120e2c65e2042be5a4875/chromeos-base/autotest-chrome/autotest-chrome-9999.ebuild [add] https://crrev.com/488cf8224808f4a0426120e2c65e2042be5a4875/chromeos-base/autotest-deps-policy/autotest-deps-policy-9999.ebuild
,
Mar 18 2018
The following revision refers to this bug: https://chromium.googlesource.com/chromiumos/third_party/autotest/+/bb2704c0645ba5084f879bc0e8355dd01a1271c6 commit bb2704c0645ba5084f879bc0e8355dd01a1271c6 Author: Lutz Justen <ljusten@chromium.org> Date: Sun Mar 18 00:22:33 2018 autotest: Refactor policy proto code Right now, every autotest that uses policy code compiles its own policy protos. There are over 30 nearly identical Makefiles for this. This CL reuses the protos generated by the central ebuild in the dependent CL and gets rid of all the Makefiles. This is much cleaner and more efficient. BUG= chromium:793333 TEST=test_that -b ${BOARD} ${DUT_IP} login_OwnershipTaken login_OwnershipApi login_OwnershipRetaken TEST=test_that -b ${BOARD} ${DUT_IP} policy_ImagesAllowedForUrls --args='case=3Urls_Allow' TEST=test_that -b ${BOARD} ${DUT_IP} suite:policy suite:bvt-perbuild TEST=cros tryjob -g 817437 -g 817436 --hwtest chell-full CQ-DEPEND=CL:817436 Change-Id: I3b9596c022f2654c0360d1253c3bfbfa648635fc Reviewed-on: https://chromium-review.googlesource.com/817437 Commit-Ready: Lutz Justen <ljusten@chromium.org> Tested-by: Lutz Justen <ljusten@chromium.org> Reviewed-by: Lutz Justen <ljusten@chromium.org> [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_CookiesBlockedForUrls/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/login_OwnershipTaken/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_ForceGoogleSafeSearch/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_NotificationsBlockedForUrls/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/login_UserPolicyKeys/src/Makefile [modify] https://crrev.com/bb2704c0645ba5084f879bc0e8355dd01a1271c6/client/site_tests/login_GuestAndActualSession/login_GuestAndActualSession.py [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_PluginsBlockedForUrls/src/Makefile [modify] https://crrev.com/bb2704c0645ba5084f879bc0e8355dd01a1271c6/client/common_lib/cros/policy.py [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_ImagesAllowedForUrls/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_PluginsAllowedForUrls/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/login_RemoteOwnership/src/Makefile [modify] https://crrev.com/bb2704c0645ba5084f879bc0e8355dd01a1271c6/client/site_tests/login_UserPolicyKeys/login_UserPolicyKeys.py [add] https://crrev.com/bb2704c0645ba5084f879bc0e8355dd01a1271c6/client/deps/policy_protos/policy_protos.py [modify] https://crrev.com/bb2704c0645ba5084f879bc0e8355dd01a1271c6/client/cros/enterprise/enterprise_policy_base.py [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_PopupsAllowedForUrls/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/login_GuestAndActualSession/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/login_MultipleSessions/src/Makefile [modify] https://crrev.com/bb2704c0645ba5084f879bc0e8355dd01a1271c6/client/site_tests/login_OwnershipRetaken/login_OwnershipRetaken.py [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_URLBlacklist/src/Makefile [modify] https://crrev.com/bb2704c0645ba5084f879bc0e8355dd01a1271c6/client/site_tests/login_MultiUserPolicy/login_MultiUserPolicy.py [modify] https://crrev.com/bb2704c0645ba5084f879bc0e8355dd01a1271c6/client/site_tests/login_OwnershipApi/login_OwnershipApi.py [modify] https://crrev.com/bb2704c0645ba5084f879bc0e8355dd01a1271c6/client/site_tests/login_OwnershipTaken/login_OwnershipTaken.py [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_ProxySettings/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_ImagesBlockedForUrls/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/enterprise_FakeEnrollment/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_RestoreOnStartupURLs/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_JavaScriptBlockedForUrls/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/login_MultiUserPolicy/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_CookiesSessionOnlyForUrls/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/enterprise_PowerManagement/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/login_OwnershipApi/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_ChromeOsLockOnIdleSuspend/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_URLWhitelist/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_JavaScriptAllowedForUrls/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_ManagedBookmarks/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/login_OwnershipRetaken/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_DisableScreenshots/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_EditBookmarksEnabled/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_CookiesAllowedForUrls/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_ForceYouTubeSafetyMode/src/Makefile [modify] https://crrev.com/bb2704c0645ba5084f879bc0e8355dd01a1271c6/client/site_tests/login_MultipleSessions/login_MultipleSessions.py [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_NotificationsAllowedForUrls/src/Makefile [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_PopupsBlockedForUrls/src/Makefile [modify] https://crrev.com/bb2704c0645ba5084f879bc0e8355dd01a1271c6/client/site_tests/enterprise_PowerManagement/enterprise_PowerManagement.py [delete] https://crrev.com/364e6eb14c413cda84900bad67ca0ef834a8c644/client/site_tests/policy_PowerManagementIdleSettings/src/Makefile [modify] https://crrev.com/bb2704c0645ba5084f879bc0e8355dd01a1271c6/client/site_tests/login_RemoteOwnership/login_RemoteOwnership.py [modify] https://crrev.com/bb2704c0645ba5084f879bc0e8355dd01a1271c6/client/cros/enterprise/enterprise_fake_dmserver.py
,
Mar 21 2018
,
Apr 25 2018
Marked verified base on code reviews and no issue with auto test results as in M67.0.3396.17 10575.13.0.
,
Apr 25 2018
|
||||||||
►
Sign in to add a comment |
||||||||
Comment 1 by vapier@chromium.org
, Dec 14 2017