This uses the new Storage Access Framework, which was required for
accessing files on the SD Card starting in android-19. But the API
was really limited until android-21, and not really complete until
android-23 or even android-26. So the levels of usability will vary a
lot based on how new the version of Android is.
ARM emulators are too slow to run these tests in a useful way. The
sad thing is that it would probably work if Android didn't put up the
ANR "Process system isn't responding" on boot each time. There seems
to be no way to increase the ANR timeout.
HttpDownloaderTest doesn't get run in gitlab-ci since it was too flaky with
internet connections in the emulator. So these were missed until I manually
ran the tests.
688057b3e7e214db49566b84d5b3dcd0db30dc2b
195aaae7e52dc1c47741965904ed17bdc816a71c
df08e84e7829652d7999eee5451080a012b00a1e
Some were removed and left removed if they were run during tests,
because the tests are supposed to be automated and the noise they added
would not have helped diagnose a failure.
Also removed the dead code around "uses-feature" which will never
get implemented, especially as it is in the XML index.
gitlab's diff views wrap badly when lines are longer than 118. Android
Studio places a grey line in the UI at 120.
@SuppressWarnings("LineLength") is added to a bunch of files to prevent
making this commit huge. People can remove that as they work on those
files, and fix the issues then.
I also ran Android Studio's default Ctrl-Alt-L code formatter, where it was
easy to do, and I was already in the file.
We've had a number of crashes due to bad formats in various
translated strings. This test runs through all of the translated
strings and tests them with the same format values that the
source strings expect. This is to ensure that the formats in the
translations are correct in number and in type (e.g. {@code s} or
{@code s}. It reads the source formats and then builds {@code
formats} to represent the position and type of the formats. Then
it runs through all of the translations with formats of the
correct number and type.
I couldn't get the Resources stuff working in Robolectric, so I
made this an emulator test.
The change to the Swedish translation included in this commit are
fixes for issues that these tests found.
closes#923
This wouldn'tve actually found the problem in the previous commit,
due to the null happening before checking permissions while logging perms.
However, still seems like a nice test to have so that the method itself
handles nulls correctly.
<uses-permissions/> tags can have min and max SDK to take effect. This is
not supported currently, and it necessary especially with the privileged
installer so it can properly represent the permissions that an APK is
requesting.
For example:
<uses-permission
android:name="android.permission.MANAGE_ACCOUNTS"
android:maxSdkVersion="22" />
<uses-permission-sdk-23
android:name="android.permission.CAMERA" />
<uses-permission-sdk-23
android:name="android.permission.CALL_PHONE"
android:maxSdkVersion="23" />
android.content.pm.PackageInfo is the Android class for representing data
about an APK/package. Since Apk.permission is the same thing, we should
use the same name.
There are oddities with the way that Android has implemented the network
stack, as compared to OpenJDK or Oracle JDK. So running the tests on the
local JVM, i.e. Robolectric, will not provide good test coverage for real
world use cases.
To appease PMD, we now have a three rulesets in `config/pmd/*.xml`:
* `rules.xml`: The bulk of the rules, used by both main and test code.
* `rules-main.xml`: Rules specific to the andoid client code.
* `rules-test.xml`: Rules specific to test code.
The rationale is because checkstyle by default checks for "too many static
imports", which is a fair call. However in JUnit4 code, it is common to
import many `assert*` static methods.
The tests pass, but there is a lingering message that gets logged:
```
Jun 08, 2016 7:31:13 AM com.almworks.sqlite4java.Internal log
WARNING: [sqlite] [DETACH DATABASE temp_update_db]DB[1][C]: exception when clearing
com.almworks.sqlite4java.SQLiteException: [1] DB[1] reset [no such database: temp_update_db]
at com.almworks.sqlite4java.SQLiteConnection.throwResult(SQLiteConnection.java:1309)
at com.almworks.sqlite4java.SQLiteConnection.throwResult(SQLiteConnection.java:1282)
at com.almworks.sqlite4java.SQLiteConnection.cacheStatementHandle(SQLiteConnection.java:1211)
at com.almworks.sqlite4java.SQLiteConnection.access$900(SQLiteConnection.java:54)
at com.almworks.sqlite4java.SQLiteConnection$CachedController.dispose(SQLiteConnection.java:1606)
at com.almworks.sqlite4java.SQLiteStatement.dispose(SQLiteStatement.java:187)
at org.robolectric.shadows.ShadowSQLiteConnection$Connections$4.call(ShadowSQLiteConnection.java:421)
at org.robolectric.shadows.ShadowSQLiteConnection$Connections$6.call(ShadowSQLiteConnection.java:449)
at org.robolectric.shadows.ShadowSQLiteConnection$Connections$6.call(ShadowSQLiteConnection.java:443)
at java.util.concurrent.FutureTask.run(FutureTask.java:266)
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
at java.lang.Thread.run(Thread.java:745)
```
The `temp_update_db` is the one used for repo updates, but I thought that it
correctly gets dropped/detached by the `TempAppProvider` when required. In fact,
given the nature of the error message (no such database: temp_update_db), that
hints at the fact that it is indeed dropped. I'm struggling to figure out what
causes this, but it should not be harmful to the running of the tests. If a test
actually fails, then it is picked up correctly by JUnit.
Many of the `Mock*` classes are there to deal with idiosyncrosies of
the Android SDK, including `final`/package local/`@Hide` annotations/etc.
They are no longer required with robolectric tests.
Get around silly `final` methods in `ContentResolver` with Mockito and `delegatesTo`.
The Robolectric library presumes that people always want to test content providers by
manually invoking the `query`/`update`/`delete` methods on the `ShadowContentResolver`.
While that is a great feature for testing, we have helper methods that require testing,
and these methods accept either a _real_ `ContentResolver` or `Context`. Robolectric
did some cool magic in terms of intercepting runtime calls to content resolvers and
forwarding them to the "shadow" verison, to deal with final/package private/etc methods.
However, as a side effect, the `ShadowContentProvider` _is not a `ContentProvider` as
far as the Java compiler is concerned.
By utilising Mockito + `delegatesTo` method, we are able to achieve what is required:
* An actual `ContentProvider` instance.
* It forwards calls to the `ShadowContentProvider` provided by Robolectric.
Robolectric provides testing support for Android via the JVM, including testing
of content providers. In order to get these tests to work, we need to avoid
the default behaviour of starting up FDroidApp.onCreate(). This method has a lot
of static state which fails if set multiple times. Instead of trying to ensure
we correctly zero out that state each test, it is preferable to instead never
bother with that in the first place. Expecially when that is not what is under
test (as is the case with content provider tests).
Since refactoring the installed app cache stuff, these methods are no longer
required for testing purposes. This is because the tests directly ask the
content provider to insert relevant apps, rather than testing the broadcast
receiving functionality.
The APK hash is useful for comparing whether something is exactly the same
file as something else. For example, to compare whether the installed APK
matches something that f-droid.org hosts. The "last update time" is a fast
way to check whether the information is current.
InstalledAppCacheUpdater was a custom Service-like thing with some
threading issues. InstalledAppProviderService is an IntentService that
relies on the built-in queue and threading of the IntentService to make
sure that things are processed nicely in the background and one at a time.
This changes the announcing so that each app added/changed/deleted triggers
a new annoucement. This keeps the UI more updated, and makes the Installed
tab show something as soon as possible, rather than waiting for the all of
the install apps to be processed. This becomes more important as more
stuff is added to InstalledAppProvider, like the hash of the APK.
This also strips down and simplifies the related BroadcastReceivers.
BroadcastReceivers work on the UI thread, so they should do as little work
as possible. PackageManagerReceiver just rebadges the incoming Intent and
sends it off to InstalledAppProviderService for processing.
Apparently, the CREATOR field is not (yet?) needed in the tests, since
they work without it. This gets us closer to making lint errors fail
the CI builds.
closes#580
A hacked fdroid server could "replay" old index.jar files known to have
apps with vulnerabilities in it. That provides a long window of time for
exploiting that vulnerability. By checking that the timestamp of an update
is never older than the current index, this attack is prevented.
The Event class is no longer needed once there is specific ProgressListener
instances for each type of progress update. The sourceUrl serves as the
unique ID, like with DownloaderService and InstallManagerService.
fixes#633https://gitlab.com/fdroid/fdroidclient/issues/633
DownloaderServiceTest is just a stub of a test that doesn't really test
anything yet. It is now causing a NullPointerException, so its a problem:
java.lang.NullPointerException
at org.fdroid.fdroid.net.DownloaderService.notifyDownloadComplete(DownloaderService.java:313)
at org.fdroid.fdroid.net.DownloaderService.handleIntent(DownloaderService.java:287)
at org.fdroid.fdroid.net.DownloaderService$ServiceHandler.handleMessage(DownloaderService.java:104)
at android.os.Handler.dispatchMessage(Handler.java:99)
at android.os.Looper.loop(Looper.java:137)
at android.os.HandlerThread.run(HandlerThread.java:60)
This moves the cache file deletion to a dedicated IntentService that runs
at the lowest possible priority. The cache cleanup does not need to happen
with any kind of priority, so it shouldn't delay the app start or take any
resources away from foreground processes.
This also changes the logic around the "Cache packages" preference. The
downloader always saves APKs, then if "Cache packages" is disabled, those
APKs are deleted when they are older than an hour.
This also simplifies Utils.deleteFiles() since the endswith arg is no
longer needed.
