[camera_android_camerax] Add agentic guidance for adding native unit tests by camsim99 · Pull Request #12369 · flutter/packages

camsim99 · 2026-08-04T21:48:25Z

Adds guidance for agents to ensure native unit tests are added when native Android code is modified by:

Updating AGENTS.md to explicitly note how to run native unit tests + that tests should be added if any of the Java files re touched
Updating the pre-push skill to ensure native unit tests are added as expected + pass
Adds evals for the new guidance added to the pre-push skill to ensure native unit tests not being added or failing causes the skill to fail. Results of running them:
- Eval 1 (Missing Native Test): PASSED – Detected that CameraProxyApi.java was modified without tests in android/src/test/ and blocked push readiness (# NO, you are not ready to push.).
- Eval 2 (Success Native Test): PASSED – Ran both dart-test and native-test --no-integration, verified CameraTest.java covered CameraProxyApi.java, handled unrelated test environment exceptions per the rubric, and approved push readiness (# YES, you are ready to push!).
- Eval 3 (Mixed Missing Native Test): PASSED – Detected that although Dart unit tests were updated for Dart changes, CameraProxyApi.java lacked a test update in android/src/test/, and blocked push readiness.
- Eval 4 (Bypassed Mixed Missing Native Test): PASSED -- Explicitly identified that CameraProxyApi.java
  was modified without a native test update, but noted: "since you explicitly approved bypassing the native test requirement for this comment-only change, this check is marked as approved."

Follow up from one-shot attempt #12302. See go/flutter-project-one-shot for more information on the project.

Pre-Review Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the gemini-code-assist bot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.

Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩²

flutter-dashboard · 2026-08-04T21:48:33Z

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

gemini-code-assist

Code Review

This pull request updates the pre-push skill documentation and agent guidelines to require and verify native Android unit tests alongside Dart unit tests, and adds evaluation scripts to test this behavior. The reviewer feedback suggests wrapping command substitutions in double quotes to handle paths with spaces, running repository-level tools from the repository root, and configuring temporary git author information in evaluation scripts to prevent commit failures in clean CI/CD environments.

reidbaker · 2026-08-06T19:48:45Z

+# Modifies a native Java file without updating a corresponding test file,
+# and commits the change to test that pre-push-skill detects missing native tests.
+
+SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"


FWIW I think I used bash so shame on me and I will make this a non blocking nit. We should have this code be dart scripts instead of bash files for team readability.

reidbaker · 2026-08-06T19:49:52Z

+# Commit so git status is clean and git diff against origin/main shows the Java change
+cd "$PACKAGE_DIR" || exit 1
+git add "$JAVA_FILE"
+git -c user.name="Author" -c user.email="author@example.com" commit -m "eval: temporary commit with Java change and no test update"


Is there a way to see if git can confirm it is in a tree and not on master. I can see agents running this eval in the wrong context and polluting the git history.

reidbaker · 2026-08-06T19:50:25Z

+JAVA_FILE="$PACKAGE_DIR/android/src/main/java/io/flutter/plugins/camerax/CameraProxyApi.java"
+
+# Add a harmless comment to CameraProxyApi.java
+echo "// Eval comment" >> "$JAVA_FILE"


a comment should not need a native test right?

reidbaker · 2026-08-06T19:53:28Z

+      "id": 1,
+      "prompt": "First, run 'bash .agents/skills/pre-push-skill/evals/test_data/setup_missing_native_test.sh'. Then, run the pre-push skill.",
+      "expected_chat_output": [
+        "The agent should detect that a native Java source file was modified without a corresponding native unit test being added or updated, and report that the code is not ready to push or ask for test coverage approval."


This expectation reads like a summary of what should happen not an expectation about what information is shared with the user.

The reason I point this out is jetski ide + gemini 3.1 pro regularly made this mistake and you and I intentionally changed the structure to have expectations for output and state.

Please change this to a list of expectations

reidbaker · 2026-08-06T19:55:15Z

+    },
+    {
+      "id": 2,
+      "prompt": "First, run 'bash .agents/skills/pre-push-skill/evals/test_data/setup_success_native_test.sh'. Then, run the pre-push skill.",


I dont know if it matters but lets use the syntax we would use in chat "run the /pre-push skill" not "run the pre-push skill"

reidbaker · 2026-08-06T19:58:15Z

Meta comment these are expensive tests to run and I predict we will want to minimize the number of these and maximize the things being evaluated per eval. Similar to an integration test.

No changed needed for now but next time we are adding evals for a particular pre-push situation lets try to pick one of the three test_data/setup_* scripts to update.

reidbaker · 2026-08-06T20:00:26Z

+TEST_FILE="$PACKAGE_DIR/android/src/test/java/io/flutter/plugins/camerax/CameraTest.java"
+
+# Modify CameraProxyApi.java and CameraTest.java
+echo "// Eval comment" >> "$JAVA_FILE"


For the code cases I think adding a new java/dart file is probably an easier to maintain and safer change for the evals. That way if it accidentally gets added to git or runs in a place we dont expect it is easy to find. Also new functionality is more likely to require a test as models get better and start reasoning.

reidbaker · 2026-08-06T20:01:06Z

+  "evals": [
+    {
+      "id": 1,
+      "prompt": "First, run 'bash .agents/skills/pre-push-skill/evals/test_data/setup_missing_native_test.sh'. Then, run the pre-push skill.",


I think you will need to add one more line that the agent should not attempt to fix any issues it fines only report the output of the skill.

reidbaker · 2026-08-06T20:02:06Z

  and [dart-collect-coverage](.agents/skills/dart-collect-coverage/SKILL.md).
 - Avoid duplicating constant strings; reuse existing ones from adjacent code.
 - **Testing Guidelines**: You MUST read and follow all rules in [TESTING.md](TESTING.md) BEFORE writing or modifying any tests. This is CRITICAL for preventing CI flakiness.
+- **Native Unit Tests**: When modifying `.java` files in `android/src/main/`, you MUST add or update corresponding native unit test files in `android/src/test/`.


Do you want to add kotlin here?

Suggested change

- **Native Unit Tests**: When modifying `.java` files in `android/src/main/`, you MUST add or update corresponding native unit test files in `android/src/test/`.

- **Native Unit Tests**: When modifying `.java` files in `android/src/main/` with logic changes, you MUST add or update corresponding native unit test files in `android/src/test/`.

Also I think you will want to exclude .g.kt files. for Pigeon

reidbaker · 2026-08-06T20:04:23Z

-  any new logic. For integration tests, see [flutter-add-integration-test](.agents/skills/flutter-add-integration-test/SKILL.md).
+  any new logic.
+  - **Dart Unit Tests**: See [dart-add-unit-test](.agents/skills/dart-add-unit-test/SKILL.md).
+  - **Native Unit Tests**: Run `cd "$(git rev-parse --show-toplevel)" && dart run script/tool/bin/flutter_plugin_tools.dart native-test --packages camera_android_camerax --no-integration`.


Do you need to run cd "$(git rev-parse --show-toplevel)" Shouldn't the flutter_plugin_tools command work no matter the directory? If we need to find flutter_plugin_tools we can modify the bootstrapping skill that runs to make sure agents are ready to work by either installing the tool or adding an environment variable.

camsim99 added 4 commits August 4, 2026 13:08

Base changes

485eb96

add evals for pre-push skill

0d7830c

Add eval for mixed Dart/native updates

6d46fea

use echo

9055c2a

flutter-dashboard Bot added the CICD Run CI/CD label Aug 4, 2026

github-actions Bot added the p: camera label Aug 4, 2026

gemini-code-assist Bot reviewed Aug 4, 2026

View reviewed changes

camsim99 added override: no versioning needed Override the check requiring version bumps for most changes override: no changelog needed Override the check requiring CHANGELOG updates for most changes labels Aug 4, 2026

camsim99 mentioned this pull request Aug 4, 2026

[camera_android_camerax] Restore torch state when camera changes #12302

Open

11 tasks

camsim99 added 3 commits August 5, 2026 10:08

address review

50ea816

add eval for bypassing adding tests check

5e91cb5

Merge remote-tracking branch 'upstream/main' into cos_nativetests

1469517

camsim99 requested a review from reidbaker August 5, 2026 17:32

camsim99 mentioned this pull request Aug 5, 2026

[camera_android_camerax] Correct pre-push skill version validation logic #12371

Open

11 tasks

reidbaker reviewed Aug 6, 2026

View reviewed changes

	- Native Unit Tests: When modifying `.java` files in `android/src/main/`, you MUST add or update corresponding native unit test files in `android/src/test/`.
	- Native Unit Tests: When modifying `.java` files in `android/src/main/` with logic changes, you MUST add or update corresponding native unit test files in `android/src/test/`.

Uh oh!

Conversation

camsim99 commented Aug 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-Review Checklist

Footnotes

Uh oh!

flutter-dashboard Bot commented Aug 4, 2026

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

camsim99 commented Aug 4, 2026 •

edited

Loading