Skip to content

Conversation

@SaintPatrck
Copy link
Contributor

🎟️ Tracking

TBD

📔 Objective

Introduce a new Claude skill named testing-android-code to provide comprehensive guidance and standardized patterns for writing tests within the Bitwarden Android codebase.

The primary objective is to codify best practices and provide Claude with quick access to reference documentation, examples, and common pitfalls, thereby improving test quality, consistency, and development efficiency.

The skill is composed of the following documents:

  • A main SKILL.md file that serves as a quick-reference guide, outlining key patterns for testing ViewModels, Compose UIs, Repositories, and Services. It also introduces test data builders and utility classes.
  • A README.md explaining the purpose and contents of the skill.
  • Detailed reference documents for:
    • test-base-classes.md: Explains the usage of BaseViewModelTest, BitwardenComposeTest, and BaseServiceTest.
    • flow-testing-patterns.md: Details how to test StateFlow and EventFlow using Turbine, highlighting the critical differences in their testing patterns.
    • critical-gotchas.md: Lists common antipatterns and mistakes, such as misusing assertCoroutineThrows with runTest and forgetting to unmock static functions.
  • Complete, copy-pasteable examples for testing major components:
    • viewmodel-test-example.md
    • compose-screen-test-example.md
    • repository-test-example.md

⏰ Reminders before review

  • Contributor guidelines followed
  • All formatters and local linters executed and passed
  • Written new unit and / or integration tests where applicable
  • Protected functional changes with optionality (feature flags)
  • Used internationalization (i18n) for all UI strings
  • CI builds passed
  • Communicated to DevOps any deployment requirements
  • Updated any necessary documentation (Confluence, contributing docs) or informed the documentation team

🦮 Reviewer guidelines

  • 👍 (:+1:) or similar for great changes
  • 📝 (:memo:) or ℹ️ (:information_source:) for notes or general info
  • ❓ (:question:) for questions
  • 🤔 (:thinking:) or 💭 (:thought_balloon:) for more open inquiry that's not quite a confirmed issue and could potentially benefit from discussion
  • 🎨 (:art:) for suggestions / improvements
  • ❌ (:x:) or ⚠️ (:warning:) for more significant problems or concerns needing attention
  • 🌱 (:seedling:) or ♻️ (:recycle:) for future improvements or indications of technical debt
  • ⛏ (:pick:) for minor or nitpick changes

@github-actions github-actions bot added the t:ci Change Type - Updates to automated workflows label Jan 15, 2026
@codecov
Copy link

codecov bot commented Jan 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 85.77%. Comparing base (b0084d2) to head (36af6b0).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6370   +/-   ##
=======================================
  Coverage   85.77%   85.77%           
=======================================
  Files         775      775           
  Lines       56203    56203           
  Branches     8123     8123           
=======================================
  Hits        48207    48207           
  Misses       5172     5172           
  Partials     2824     2824           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@github-actions
Copy link
Contributor

github-actions bot commented Jan 15, 2026

Logo
Checkmarx One – Scan Summary & Details8862ae5d-eaaf-454f-8e79-b2125a9b4bce

Great job! No new security vulnerabilities introduced in this pull request

@SaintPatrck SaintPatrck force-pushed the claude/testing-android-code-skill branch 3 times, most recently from ebe7698 to f83fdfc Compare January 29, 2026 17:38
This commit introduces a new Claude skill named `testing-android-code` to provide comprehensive guidance and standardized patterns for writing tests within the Bitwarden Android codebase.

The primary objective is to codify best practices and provide developers with quick access to reference documentation, examples, and common pitfalls, thereby improving test quality, consistency, and development efficiency.

The skill is composed of the following documents:
- A main `SKILL.md` file that serves as a quick-reference guide, outlining key patterns for testing ViewModels, Compose UIs, Repositories, and Services. It also introduces test data builders and utility classes.
- A `README.md` explaining the purpose and contents of the skill.
- Detailed reference documents for:
    - `test-base-classes.md`: Explains the usage of `BaseViewModelTest`, `BitwardenComposeTest`, and `BaseServiceTest`.
    - `flow-testing-patterns.md`: Details how to test StateFlow and EventFlow using Turbine, highlighting the critical differences in their testing patterns.
    - `critical-gotchas.md`: Lists common antipatterns and mistakes, such as misusing `assertCoroutineThrows` with `runTest` and forgetting to unmock static functions.
- Complete, copy-pasteable examples for testing major components:
    - `viewmodel-test-example.md`
    - `compose-screen-test-example.md`
    - `repository-test-example.md`
@SaintPatrck SaintPatrck force-pushed the claude/testing-android-code-skill branch from f83fdfc to db99690 Compare January 29, 2026 17:49
@SaintPatrck SaintPatrck marked this pull request as ready for review January 29, 2026 17:50
@SaintPatrck SaintPatrck requested a review from a team as a code owner January 29, 2026 17:50
@SaintPatrck SaintPatrck requested review from a team and david-livefront January 29, 2026 17:51
@SaintPatrck SaintPatrck added the ai-review Request a Claude code review label Jan 29, 2026
@withinfocus
Copy link
Contributor

https://bitwarden.slack.com/archives/C09MHQNCSKD/p1769693094966979 -- is this a skill or a prompt? And do we think this will move to the marketplace in time or is so specific it will just be here? I am thinking about the reviewer agent benefitting.

@SaintPatrck
Copy link
Contributor Author

SaintPatrck commented Jan 29, 2026

@withinfocus it's a Skill, although I'm toying with the idea of making it a Rule once I've had a chance to tinker with them.

I keep going back-and-forth on moving it to the marketplace or keeping it here given it's so specialized. My main hesitation is the friction it may introduce for maintenance. If we consider it a living document to update as we fine-tune Claude for this repo, I would prefer it reside here. I feel like that will encourage and facilitate faster iterations.

The reviewer agent, and the implementor/developer agent being built, will inherently have access to this as long as they're initiated in the repo root. If we move it to the marketplace the plugin has to be manually installed everywhere. There may be ways to configure the repo to automagically install the marketplace+plugin, but I've not looked into that approach thoroughly.

For the sake of getting something started I think it's best we begin with it here, iterate a bit until the team is confident with it, then promote it to the marketplace if there's a larger desire/need to do so.

Edit:
With all of that being said, I just remembered we have at least one other Android repo. If it follows the guidelines and best practices we've established in this repo, that's a good enough reason for me to promote it to the marketplace sooner rather than later.

}

companion object {
private val DEFAULT_STATE = ExampleState(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we move this out of the companion object and just have it be at the top-level of the file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

}

// Default state for assertions - always define complete expected states
private val defaultState = ExampleState(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stuff like this is usually top-level

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

..and done.

private fun createViewModel(
initialState: ExampleState? = null,
): ExampleViewModel = ExampleViewModel(
savedStateHandle = SavedStateHandle().apply { set("state", initialState) },
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Moves the `DEFAULT_STATE` constant from a class-level property to a top-level private val in both `viewmodel-test-example.md` and `compose-screen-test-example.md`.

This change improves consistency and cleans up the test class structure by removing the `companion object` in the Compose test example.
@claude
Copy link
Contributor

claude bot commented Jan 29, 2026

Claude finished @SaintPatrck's task in 1m 38s —— View job


Code Review Summary

PR: Add Android testing skill for Claude

Overview

This PR introduces a comprehensive Claude skill for testing Android code in the Bitwarden codebase. The skill provides tactical testing guidance including base class documentation, flow testing patterns, and common anti-patterns.

Files Reviewed

File Assessment
README.md Documentation overview
SKILL.md Core reference patterns
examples/viewmodel-test-example.md Complete ViewModel test example
examples/compose-screen-test-example.md Complete Compose screen test
examples/repository-test-example.md Complete repository test with Fake/Mock patterns
references/test-base-classes.md Base class documentation
references/flow-testing-patterns.md Turbine StateFlow/EventFlow patterns
references/critical-gotchas.md Anti-patterns and debugging tips

Verification

  • All referenced file paths exist in the codebase (BaseViewModelTest, BitwardenComposeTest, BaseServiceTest, FakeDispatcherManager, etc.)
  • Code examples follow established Bitwarden testing conventions
  • Prior review feedback (moving DEFAULT_STATE to top-level) has been addressed in the latest commit

Assessment

No issues found. This is a well-organized documentation skill that:

  • Accurately documents existing testing patterns
  • Provides practical, copy-pasteable examples
  • Covers critical gotchas that commonly cause test failures
  • Uses correct file references verified against the codebase

Automated review by Claude

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-review Request a Claude code review t:ci Change Type - Updates to automated workflows

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants