Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions .githooks/pre-commit
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,16 @@ if [[ -n "$changed_python" ]]; then
fi
fi

# ---------------------------------------------------------------------------------------
# Java: fully qualified names where an import would do. The whole tree rather than the staged
# files, because whether a long name is needed depends on the other classes in its package.
# Under a second, and it reads source only.
# ---------------------------------------------------------------------------------------
if staged | grep -qE '^app/src/.*\.java$'; then
say "Java changed - checking for fully qualified names an import would do"
"$PYTHON" scripts/check_java_imports.py || failed=1
fi

# ---------------------------------------------------------------------------------------
# Secrets. secrets.properties holds a real Google Maps key and is gitignored; committing it
# would publish the key, and a published key cannot be unpublished, only rotated.
Expand Down
7 changes: 2 additions & 5 deletions .github/workflows/static-checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -58,12 +58,9 @@ jobs:
python -m pip install pytest
python -m pytest scripts/test -v

# Reported but not yet blocking: the existing code has about 270 findings, and the cleanup
# (`--fix`) waits until the open pull requests are merged so it does not conflict with them.
# Remove continue-on-error once it is done. See "Fully qualified names in Java" in
# CONTRIBUTING.md.
# A long name only where the short one would clash. See "Fully qualified names in Java"
# in CONTRIBUTING.md; `--fix` rewrites them.
- name: Check Java uses imports, not fully qualified names
continue-on-error: true
run: python scripts/check_java_imports.py

# pyright rather than mypy: the editors used on this project run Pylance, which is
Expand Down
5 changes: 3 additions & 2 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -992,8 +992,9 @@ locales the first time it was run. It also runs in CI, on every PR and every rel

### Pre-commit hook

`.githooks/pre-commit` runs the translation check, plus flake8 and pyright over whichever
Python files are staged, and refuses a commit that stages `secrets.properties`. Hooks are not
`.githooks/pre-commit` runs the translation check, flake8 and pyright over whichever Python
files are staged, the Java import check when Java is staged, and refuses a commit that stages
`secrets.properties`. Hooks are not
versioned, so each clone has to opt in once:

```bash
Expand Down
12 changes: 5 additions & 7 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,8 @@ Hooks are not versioned, so this is once per clone:
git config core.hooksPath .githooks
```

`.githooks/pre-commit` runs the translation check, plus flake8 and pyright over whatever
Python you staged, and refuses to commit `secrets.properties`. It deliberately does not
`.githooks/pre-commit` runs the translation check, flake8 and pyright over whatever Python you
staged, the Java import check when you staged Java, and refuses to commit `secrets.properties`. It deliberately does not
build or run tests — a hook that takes a minute gets bypassed with `--no-verify` within a
day, and a routinely-bypassed hook is worse than none. Use `--no-verify` when you genuinely
need to.
Expand Down Expand Up @@ -689,10 +689,8 @@ python scripts/check_java_imports.py --fix # shorten them and add the imports
```

It reads the source alone, so it needs no Gradle and runs in under a second. It runs in
`static-checks.yml` but **does not fail the build yet**: the existing code has about 270 of these,
and fixing them is being held back until the open pull requests are in, so the cleanup does not
conflict with them. Once it is fixed, the step becomes blocking.

`static-checks.yml` and fails the build, and the pre-commit hook runs it whenever a Java file is
staged.
---

## Adding user-facing strings (Coding Agents)
Expand Down Expand Up @@ -877,7 +875,7 @@ run regardless: each AES entry carries a fresh random salt.

| Workflow | When | What |
| --- | --- | --- |
| `static-checks.yml` | `app/**`, `scripts/**` changes | Translation check, pyright, tooling tests, Java import check (reported, not yet blocking). Seconds, no Android |
| `static-checks.yml` | `app/**`, `scripts/**` changes | Translation check, pyright, tooling tests, Java import check. Seconds, no Android |
| `build-debug.yml` | Android build inputs change | Instrumented tests on an emulator, JVM tests, Chaquopy bridge tests, debug APK. Also publishes the generated Room schemas as `room-schemas`, so a version bump made without an Android toolchain can still commit the JSON rule 1 requires |
| `build-release.yml` | on release | Translation check, JVM tests, Chaquopy bridge tests, release APK |
| `macos-scripts-python.yml` | `python/**` changes | Exporter and shared-package tests across Python 3.10–3.13 on macOS 14 |
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,10 @@
import dev.wander.android.opentagviewer.python.FakeAppleAuthService;
import dev.wander.android.opentagviewer.python.TermsDocument;
import dev.wander.android.opentagviewer.util.android.AppCryptographyUtil;
import android.view.View;
import android.widget.ScrollView;
import androidx.test.espresso.Espresso;
import androidx.test.espresso.action.ViewActions;

import static androidx.test.espresso.intent.Intents.intending;
import static androidx.test.espresso.intent.matcher.IntentMatchers.hasComponent;
Expand Down Expand Up @@ -261,7 +265,7 @@ public void thesystemBackButtonNeitherCrashesNorAgrees() {
Eventually.check(() -> onView(withId(R.id.login_terms_container))
.check(matches(isDisplayed())));

androidx.test.espresso.Espresso.pressBack();
Espresso.pressBack();

// Still here, still on the first document, and nothing sent.
Eventually.check(() -> onView(withId(R.id.login_terms_container))
Expand Down Expand Up @@ -308,7 +312,7 @@ public void thedocumentScrollsWhenItIsLongerThanTheBox() {
for (int attempt = 0; attempt < 30 && scrollY() != previous; attempt++) {
previous = scrollY();
onView(withId(R.id.login_terms_scroll))
.perform(androidx.test.espresso.action.ViewActions.swipeUp());
.perform(ViewActions.swipeUp());
// Paced so this is watchable when somebody is being shown the flow. Off by default,
// so a normal run and CI are unaffected.
TestPace.afterAStep();
Expand Down Expand Up @@ -342,7 +346,7 @@ private static String aVeryLongDocument() {
private int scrollY() {
final int[] value = {-1};
this.scenario.onActivity(activity ->
value[0] = ((android.widget.ScrollView)
value[0] = ((ScrollView)
activity.findViewById(R.id.login_terms_scroll)).getScrollY());
return value[0];
}
Expand All @@ -364,7 +368,7 @@ private int contentHeight() {
private boolean isShown(final int id) {
final boolean[] shown = {false};
this.scenario.onActivity(activity ->
shown[0] = activity.findViewById(id).getVisibility() == android.view.View.VISIBLE);
shown[0] = activity.findViewById(id).getVisibility() == View.VISIBLE);
return shown[0];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@
import dev.wander.android.opentagviewer.python.PythonAuthService.AuthMethodPhone;
import dev.wander.android.opentagviewer.util.HowLongToWait;
import dev.wander.android.opentagviewer.util.android.AppCryptographyUtil;
import android.content.Context;
import java.util.Locale;

/**
* Signing in, driven end to end with Apple replaced.
Expand Down Expand Up @@ -335,7 +337,7 @@ public void appleDecliningIsNotBlamedOnThePasswordOrTheNetwork() {
launch();
signIn();

final android.content.Context context = getInstrumentation().getTargetContext();
final Context context = getInstrumentation().getTargetContext();

Eventually.check(() -> onView(withId(R.id.login_error_message_text)).check(matches(
withText(context.getString(R.string.login_failed_apple_declined)))));
Expand Down Expand Up @@ -370,7 +372,7 @@ public void icloudBeingRefusedIsNotReportedAsTermsOfService() {
launch();
signIn();

final android.content.Context context = getInstrumentation().getTargetContext();
final Context context = getInstrumentation().getTargetContext();

Eventually.check(() -> onView(withId(R.id.login_error_message_text)).check(matches(
withText(context.getString(R.string.login_failed_icloud_refused)))));
Expand Down Expand Up @@ -402,11 +404,11 @@ public void aWaitAppleNamedIsShownRoundedUp() {
launch();
signIn();

final android.content.Context context = getInstrumentation().getTargetContext();
final java.util.Locale locale = context.getResources().getConfiguration().getLocales().get(0);
final Context context = getInstrumentation().getTargetContext();
final Locale locale = context.getResources().getConfiguration().getLocales().get(0);

// The device's ICU, not a string this test composed: 90 seconds is two minutes, not one.
assertEquals("2 minutes", HowLongToWait.of(90).describe(java.util.Locale.ENGLISH));
assertEquals("2 minutes", HowLongToWait.of(90).describe(Locale.ENGLISH));

Eventually.check(() -> onView(withId(R.id.login_error_message_text)).check(matches(
withText(context.getString(R.string.login_failed_apple_asked_to_wait,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,12 @@
import dev.wander.android.opentagviewer.db.AccountBeaconsForTests;
import dev.wander.android.opentagviewer.python.AppDependencies;
import dev.wander.android.opentagviewer.python.icloud.FakeICloudService;
import android.view.View;
import androidx.test.platform.app.InstrumentationRegistry;
import dev.wander.android.opentagviewer.db.datastore.UserAuthDataStore;
import dev.wander.android.opentagviewer.db.repo.KeychainMembershipRepository;
import dev.wander.android.opentagviewer.util.android.AppCryptographyUtil;
import org.junit.Before;

/**
* What back does on each step of reading the account.
Expand All @@ -51,7 +57,7 @@ public void putTheRealOneBack() {
AccountBeaconsForTests.forgetThemAll();
}

@org.junit.Before
@Before
public void forgetAnyStoredMembership() {
// Finishing this flow writes real rows into the real database. Cleared here too,
// because a test that crashed left its tags behind for whatever runs next.
Expand All @@ -61,11 +67,11 @@ public void forgetAnyStoredMembership() {
// FetchFromICloudMembershipTest stores one; without this, every test here resumes as a
// member and never sees the device list. Cleared before rather than only after, because
// a test that crashed leaves it behind.
new dev.wander.android.opentagviewer.db.repo.KeychainMembershipRepository(
dev.wander.android.opentagviewer.db.datastore.UserAuthDataStore.getInstance(
androidx.test.platform.app.InstrumentationRegistry.getInstrumentation()
new KeychainMembershipRepository(
UserAuthDataStore.getInstance(
InstrumentationRegistry.getInstrumentation()
.getTargetContext()),
new dev.wander.android.opentagviewer.util.android.AppCryptographyUtil())
new AppCryptographyUtil())
.forget().blockingAwait();
}

Expand All @@ -79,7 +85,7 @@ private void open(final FakeICloudService fake) {
private boolean isShown(final int id) {
final boolean[] shown = {false};
this.scenario.onActivity(activity ->
shown[0] = activity.findViewById(id).getVisibility() == android.view.View.VISIBLE);
shown[0] = activity.findViewById(id).getVisibility() == View.VISIBLE);
return shown[0];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@
import dev.wander.android.opentagviewer.python.icloud.FakeICloudService;
import dev.wander.android.opentagviewer.python.icloud.ICloudException;
import dev.wander.android.opentagviewer.python.icloud.ICloudFailure;
import android.view.View;
import androidx.test.platform.app.InstrumentationRegistry;
import dev.wander.android.opentagviewer.db.datastore.UserAuthDataStore;
import dev.wander.android.opentagviewer.db.repo.KeychainMembershipRepository;
import dev.wander.android.opentagviewer.util.android.AppCryptographyUtil;
import org.junit.Before;

/**
* What the user is told when reading the account does not work.
Expand Down Expand Up @@ -54,7 +60,7 @@ public void putTheRealOneBack() {
AccountBeaconsForTests.forgetThemAll();
}

@org.junit.Before
@Before
public void forgetAnyStoredMembership() {
// Finishing this flow writes real rows into the real database. Cleared here too,
// because a test that crashed left its tags behind for whatever runs next.
Expand All @@ -64,11 +70,11 @@ public void forgetAnyStoredMembership() {
// FetchFromICloudMembershipTest stores one; without this, every test here resumes as a
// member and never sees the device list. Cleared before rather than only after, because
// a test that crashed leaves it behind.
new dev.wander.android.opentagviewer.db.repo.KeychainMembershipRepository(
dev.wander.android.opentagviewer.db.datastore.UserAuthDataStore.getInstance(
androidx.test.platform.app.InstrumentationRegistry.getInstrumentation()
new KeychainMembershipRepository(
UserAuthDataStore.getInstance(
InstrumentationRegistry.getInstrumentation()
.getTargetContext()),
new dev.wander.android.opentagviewer.util.android.AppCryptographyUtil())
new AppCryptographyUtil())
.forget().blockingAwait();
}

Expand All @@ -81,7 +87,7 @@ private void open(final FakeICloudService fake) {
private boolean isShown(final int id) {
final boolean[] shown = {false};
this.scenario.onActivity(activity ->
shown[0] = activity.findViewById(id).getVisibility() == android.view.View.VISIBLE);
shown[0] = activity.findViewById(id).getVisibility() == View.VISIBLE);
return shown[0];
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,13 @@
import dev.wander.android.opentagviewer.ui.compat.TheNavigationBar;
import dev.wander.android.opentagviewer.python.icloud.FakeICloudService;
import dev.wander.android.opentagviewer.python.icloud.ICloudService;
import android.content.Context;
import dev.wander.android.opentagviewer.anisette.LocalAnisette;
import dev.wander.android.opentagviewer.db.datastore.UserAuthDataStore;
import dev.wander.android.opentagviewer.db.repo.KeychainMembershipRepository;
import dev.wander.android.opentagviewer.util.android.AppCryptographyUtil;
import org.hamcrest.Matchers;
import org.junit.Before;

/**
* Reading the account, driven end to end with iCloud replaced.
Expand Down Expand Up @@ -69,7 +76,7 @@ public void putTheRealOneBack() {
AccountBeaconsForTests.forgetThemAll();
}

@org.junit.Before
@Before
public void forgetAnyStoredMembership() {
// Finishing this flow writes real rows into the real database. Cleared here too,
// because a test that crashed left its tags behind for whatever runs next.
Expand All @@ -79,11 +86,11 @@ public void forgetAnyStoredMembership() {
// FetchFromICloudMembershipTest stores one; without this, every test here resumes as a
// member and never sees the device list. Cleared before rather than only after, because
// a test that crashed leaves it behind.
new dev.wander.android.opentagviewer.db.repo.KeychainMembershipRepository(
dev.wander.android.opentagviewer.db.datastore.UserAuthDataStore.getInstance(
androidx.test.platform.app.InstrumentationRegistry.getInstrumentation()
new KeychainMembershipRepository(
UserAuthDataStore.getInstance(
InstrumentationRegistry.getInstrumentation()
.getTargetContext()),
new dev.wander.android.opentagviewer.util.android.AppCryptographyUtil())
new AppCryptographyUtil())
.forget().blockingAwait();
}

Expand All @@ -98,17 +105,17 @@ public void forgetAnyStoredMembership() {
*/
private static final String THE_STORED_SERIAL = "0PENTAGVK7QX";

@org.junit.Before
@Before
public void giveThisInstallASerialToShow() {
final android.content.Context context =
final Context context =
InstrumentationRegistry.getInstrumentation().getTargetContext();

context.getSharedPreferences(
dev.wander.android.opentagviewer.anisette.LocalAnisette.PREFERENCES,
android.content.Context.MODE_PRIVATE)
LocalAnisette.PREFERENCES,
Context.MODE_PRIVATE)
.edit()
.putString(
dev.wander.android.opentagviewer.anisette.LocalAnisette.KEY_SERIAL,
LocalAnisette.KEY_SERIAL,
THE_STORED_SERIAL)
.commit();
}
Expand All @@ -118,10 +125,10 @@ public void giveThisInstallASerialToShow() {
public void takeTheSerialBackOut() {
InstrumentationRegistry.getInstrumentation().getTargetContext()
.getSharedPreferences(
dev.wander.android.opentagviewer.anisette.LocalAnisette.PREFERENCES,
android.content.Context.MODE_PRIVATE)
LocalAnisette.PREFERENCES,
Context.MODE_PRIVATE)
.edit()
.remove(dev.wander.android.opentagviewer.anisette.LocalAnisette.KEY_SERIAL)
.remove(LocalAnisette.KEY_SERIAL)
.commit();
}

Expand Down Expand Up @@ -158,7 +165,7 @@ private void typeThePasscode() {
private boolean isShown(final int id) {
final boolean[] shown = {false};
this.scenario.onActivity(activity ->
shown[0] = activity.findViewById(id).getVisibility() == android.view.View.VISIBLE);
shown[0] = activity.findViewById(id).getVisibility() == View.VISIBLE);
return shown[0];
}

Expand Down Expand Up @@ -407,8 +414,8 @@ public void aserviceHavingABadDayOffersARetryInstead() {
public void appleDecliningSaysSoRatherThanShowingTheStatusCode() {
this.open(FakeICloudService.whereAppleIsDeclining());

final android.content.Context context =
androidx.test.platform.app.InstrumentationRegistry
final Context context =
InstrumentationRegistry
.getInstrumentation().getTargetContext();

Eventually.check(() -> onView(withId(R.id.icloud_retry_container))
Expand All @@ -417,7 +424,7 @@ public void appleDecliningSaysSoRatherThanShowingTheStatusCode() {
withText(context.getString(R.string.icloud_apple_declined_body)))));

onView(withId(R.id.icloud_retry_body)).check(matches(
not(withText(org.hamcrest.Matchers.containsString("503")))));
not(withText(Matchers.containsString("503")))));
onView(withId(R.id.icloud_no_tags_container)).check(matches(not(isDisplayed())));
TestPace.afterAStep();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import dev.wander.android.opentagviewer.python.icloud.EscrowPasscode;
import dev.wander.android.opentagviewer.python.icloud.FakeICloudService;
import dev.wander.android.opentagviewer.util.android.AppCryptographyUtil;
import android.view.View;

/**
* Asking for the device passcode once, and never again.
Expand Down Expand Up @@ -82,7 +83,7 @@ private void open(final FakeICloudService fake) {
private boolean isShown(final int id) {
final boolean[] shown = {false};
this.scenario.onActivity(activity ->
shown[0] = activity.findViewById(id).getVisibility() == android.view.View.VISIBLE);
shown[0] = activity.findViewById(id).getVisibility() == View.VISIBLE);
return shown[0];
}

Expand Down
Loading
Loading