Update & re-enable tests for the IntentResolver package.
(The "updates" were made in a later snapshot to a CL that originally
*just* re-enabled the tests. See the end of this message for a
description of the additional changes that were made.)
The entire test suite had been disabled for the new (so-called)
"unbundled" Chooser package by ag/19582316, in response to flaky test
timeouts that appeared in b/241944046.
We may(?) not have taken explicit steps to address the root cause of
those flakes (and certainly don't do so in this CL), so there's a
chance that they'll re-appear at some point after the tests are
enabled. On the other hand, the set of test cases enabled in
UnbundledChooserActivityTest after this CL is the same as those
currently enabled for framework, and if there's any divergence
in the test behavior between these two packages, we'll really need to
investigate that as a possible regression caused by the package
change.
I ran the tests 50x to check for possible flakes:
http://ab/I63200010093031013
Note this CL still leaves some planned testing-related work out of
scope, to re-work and re-enable tests that had been disabled even in
framework (b/208803748), and to remove the scaffolding that we used to
share tests in the previous inheritance-based model (b/248567224).
UPDATES SINCE ORIGINAL SNAPSHOT:
1. Remove dependency on framework ChooserActivityTestsLib, which
imported a copy of the framework ChooserActivityTests into the
IntentResolverUnitTests (in addition to supporting the old
inheritance/test-sharing model). Previously my local testing only
included UnbundledChooserActivityTests, and the earlier snapshot
failed presubmits as a result of the other ("imported copy") tests
that ran against the framework implementation (which is no longer a
concern of ours post-fork, but regressed only in the
UnbundledChooserActivityTests "copy" after a change made in
ag/20001357). Note this also required forking over a single test-only
resource that was previously shared.
2. Refactor a helper method to set DeviceConfig overrides in the
test. This was the result of chasing down a red herring in the failing
IntentResolverUnitTests; I didn't realize the WRITE_DEVICE_CONFIG
permission-checks were only failing in the "imported copy" tests and
thought there was some problem with our usage of
adoptShellPermissionIdentity() which only came up when running
remotely. The helper is based on one with the same name that occurs
several times in codesearch, which usually also manages the permission
overrides needed for the call. I ultimately removed the permission
management responsibilities to avoid any possible conflicts with other
such logic we'd already set up for our tests, but I left the general
structure for improved readability (with some aspiration that we might
eventually switch to managing permissions in the style of the other
examples that use this helper).
3. Remove lookups of framework "runtime resource IDs" throughout the
tests. This practice was always suspect, and it's not appropriate after
the fork (when many of the resources have actually migrated to the new
package and can't necessarily be located by their old framework IDs;
OTOH it's also less important after the fork, because there's a smaller
surface for shared resources to diverge between changes). This broke
locally while I was investigating the need for the other changes, but it
seemed important to fix as part of this CL (since otherwise our tests
might only pass "by coincidence" if the resource IDs align). I still
don't fully understand the requirements but I wonder if issues around
this would be related to the "binary compatibility" mentioned in the
framework resources values/public-final.xml -- that might motivate us to
prioritize cleanup of any other shared resources where we can't rely on
that compatibility (b/249076383 etc)
Test: atest --flakes-info --iterations 50 IntentResolverUnitTests
Bug: 241944046, 248567224
Change-Id: I77826343e60d612111e04997c8e52326a2fee6d5
4 files changed
