Docs updates - introduce Running Tests, introduce Web UI Frontend, small fixes (#35231)
* Docs updates - New test suites doc (how to run tests) - New WebUI Frontend doc (basic introduction to Brave's different stack) - Patches points to best practices - bug fix in claude code skills doc - Adjust docs/README.md to prioritize the index - Preflight skill addition of missing test suites
This commit is contained in:
Pete Miller
+30
-23
@@ -4,29 +4,6 @@ This directory contains documentation for the Brave Browser. For additional reso
|
||||
|
||||
* [brave-browser/wiki](https://github.com/brave/brave-browser/wiki)
|
||||
|
||||
> [!IMPORTANT]
|
||||
> If you add new documents, please also add a link to them in the Document
|
||||
> Index below.
|
||||
|
||||
## Creating Documentation
|
||||
|
||||
### Guidelines
|
||||
|
||||
* Markdown documents must follow the
|
||||
[Markdown Style
|
||||
Guide](https://chromium.googlesource.com/chromium/src/+/HEAD/styleguide/markdown/markdown.md).
|
||||
|
||||
### Previewing changes
|
||||
|
||||
#### Locally using [md_browser](https://chromium.googlesource.com/chromium/src/+/refs/heads/main/tools/md_browser/)
|
||||
|
||||
```bash
|
||||
# in src/brave/
|
||||
npm run docs
|
||||
```
|
||||
|
||||
This is only an estimate. The **github** view may differ.
|
||||
|
||||
## Document Index
|
||||
|
||||
### Checking Out and Patching
|
||||
@@ -49,6 +26,8 @@ This is only an estimate. The **github** view may differ.
|
||||
beyond standard Chromium encryption.
|
||||
|
||||
### General Development
|
||||
* [Running test suites](running_test_suites.md) - Selectively execute unit,
|
||||
browser and typescript tests.
|
||||
* [`gni` notes](gni_sources.md) - Brief notes on the use of `source.gni`
|
||||
files in our code base.
|
||||
* [Rust notes](rust.md) - General recommendations on integrating rust code
|
||||
@@ -63,3 +42,31 @@ This is only an estimate. The **github** view may differ.
|
||||
How to exclude files from format and presubmit checks.
|
||||
* [Siso Customization](siso_customization.md) - How we customize Siso to work
|
||||
for us.
|
||||
* [WebUI Frontend](./webui_frontend.md) - General Web UI, React, webpack
|
||||
overview.
|
||||
* [WebUI Testing](./webui_testing.md) - Guide to creating and running *.test.ts(x) suites.
|
||||
* [WebUI Strings Overview](./webui_strings_explainer.md) - Including strings in a Web UI frontend.
|
||||
|
||||
|
||||
## Creating Documentation
|
||||
|
||||
> [!IMPORTANT]
|
||||
> If you add new documents, please also add a link to them in the Document
|
||||
> Index below.
|
||||
|
||||
### Guidelines
|
||||
|
||||
* Markdown documents must follow the
|
||||
[Markdown Style
|
||||
Guide](https://chromium.googlesource.com/chromium/src/+/HEAD/styleguide/markdown/markdown.md).
|
||||
|
||||
### Previewing changes
|
||||
|
||||
#### Locally using [md_browser](https://chromium.googlesource.com/chromium/src/+/refs/heads/main/tools/md_browser/)
|
||||
|
||||
```bash
|
||||
# in src/brave/
|
||||
npm run docs
|
||||
```
|
||||
|
||||
This is only an estimate. The **github** view may differ.
|
||||
|
||||
@@ -2,7 +2,7 @@
|
||||
|
||||
Claude Code skills are slash commands that automate common development tasks
|
||||
in `brave-core`. Skills are defined in `.claude/skills/` and best practices
|
||||
in `.claude/rules/best-practices/`.
|
||||
in `docs/best-practices/`.
|
||||
|
||||
## Available Skills
|
||||
|
||||
|
||||
@@ -89,34 +89,19 @@ No BUILD.gn changes are needed for this.
|
||||
|
||||
### Patch the Chromium files
|
||||
|
||||
When other options are exhausted, you can patch the code directly in `src/`. After making the changes, you can run the npm command `npm run update_patches`. This will update the patches which are stored in `src/brave/patches`. Please note that removed changes in `src` currently will not update the patches, so you will have to do that manually.
|
||||
When other options are exhausted, you can patch the code directly in `src/`. After making the changes, you can run the npm command `npm run update_patches`. This will update the patches which are stored in `src/brave/patches`. Please note that removed changes in `src` currently will not update the patches, so you will have to do that manually.
|
||||
|
||||
.patchinfo files are git-ignored metadata files to track which patches have been
|
||||
applied. They are created during `npm run apply_patches`. Run that command
|
||||
whenever switching branches where .patch files are added, modified, or removed
|
||||
so that chromium files get updated. Sync will also run that command, so you
|
||||
don't have to yourself if performing that.
|
||||
|
||||
_Tip: Directly after generating new patches and when changing branches where patches are modified, ensure you run `npm run apply_patches` to generate the metadata and remove your patched changes when changing to a branch no longer containing the patches._
|
||||
|
||||
We aim to make the only patches required to be trivial changes, and not nested logic changes.
|
||||
If possible write the patch to add a new line vs appending/prepending to an existing line.
|
||||
|
||||
For example, instead of
|
||||
```
|
||||
- return !url.is_empty() && !url.SchemeIs(content::kChromeUIScheme) &&
|
||||
+ return IsBraveTranslateEnabled() && !url.is_empty() && !url.SchemeIs(content::kChromeUIScheme) &&
|
||||
!url.SchemeIs(content::kChromeDevToolsScheme) &&
|
||||
```
|
||||
it should be
|
||||
```
|
||||
return !url.is_empty() && !url.SchemeIs(content::kChromeUIScheme) &&
|
||||
+ IsBraveTranslateEnabled() &&
|
||||
!url.SchemeIs(content::kChromeDevToolsScheme) &&
|
||||
```
|
||||
Do not add comments in patches and ignore lint line length rules to squash patches onto one line whenever possible
|
||||
|
||||
You should almost never patch in two methods calls in a row. We should prefer extensible patches. For instance https://github.com/brave/brave-core/pull/2693/files#diff-a9c9a8da7aa4df821394352a0ca04a27R12:
|
||||
```
|
||||
CopyBraveExtensionLocalization(config, staging_dir, g_archive_inputs)
|
||||
CopyBraveRewardsExtensionLocalization(config, staging_dir, g_archive_inputs)
|
||||
```
|
||||
inside `CopyAllFilesToStagingDir` would be collapsed to
|
||||
```
|
||||
CopyBraveFilesToStagingDir
|
||||
```
|
||||
See [best-practices/patches.md](best-practices/patches.md) for important criteria.
|
||||
|
||||
Make sure you do NOT have the following in your `~/.gitconfig`:
|
||||
|
||||
|
||||
@@ -0,0 +1,23 @@
|
||||
# Running tests
|
||||
|
||||
Usually you'll run:
|
||||
- `npm run test -- [test suite] --filter="..."`
|
||||
|
||||
where test suite is any of:
|
||||
|
||||
- brave_components_unittests (C++ unit tests in brave/components)
|
||||
- brave_unit_tests (C++ unit tests anywhere else in brave/)
|
||||
- brave_browser_tests
|
||||
- chromium_unit_tests (Chromium C++ unit tests in src/)
|
||||
- browser_tests (Chromium browser tests in src/)
|
||||
|
||||
and `--filter` is the full name of the test (FixtureName.TestName) but you can
|
||||
use wildcards and combine filters with `:` e.g. `--filter="*Foo*:*Bar*"`
|
||||
|
||||
See chromium's docs/testing_in_chromium.md.
|
||||
|
||||
## Javascript / Typescript unit tests
|
||||
|
||||
Isolated *.test.ts(x) tests are run via:
|
||||
|
||||
- npm run test-unit -- [path blob filter]
|
||||
@@ -0,0 +1,37 @@
|
||||
# Web UI Frontend
|
||||
|
||||
For most brave-specific WebUI, a different stack to Chromium is used for the
|
||||
frontend.
|
||||
|
||||
- TypeScript/React components often in `components/*/resources/` or
|
||||
`components/*_ui` or `browser/ui/webui/resources/*`
|
||||
- Webpack configuration in `components/webpack/`
|
||||
- Uses Storybook for component development and user, designer, and reviewer
|
||||
verification
|
||||
- Tests via jest in sibling *.test.ts(x) files
|
||||
|
||||
## Storybook
|
||||
|
||||
All WebUI should have a Storybook. This allows:
|
||||
- Rapid realtime iteration with hot-reload
|
||||
- Links to be generated by CI during PRs and shared with other team members
|
||||
|
||||
This needs UIs to be:
|
||||
- Built with a componentized mindset - data has to be mocked for tests, for storybook, or provided by the real WebUI communications layer (usually mojom).
|
||||
|
||||
## State
|
||||
|
||||
Consider using [createInterfaceApi](../components/common/api/readme.md) to wrap
|
||||
mojom interfaces and generate React hooks as well as create a layer that is easily mocked.
|
||||
|
||||
For local UI-only state, keep it as simple and local as possible whilst avoiding both prop-drilling and unneccessary re-renders. Consider using small Contexts for components that all need all of the same state properties or computed values, or
|
||||
an external store, with individual property-level subscription, only when disparate components need access to some of the same state or computed values.
|
||||
|
||||
## Testing
|
||||
See [WebUI Testing](./webui_testing.md)
|
||||
|
||||
## Strings
|
||||
See [WebUI Strings](./webui_strings_explainer.md)
|
||||
|
||||
## Chromium
|
||||
See [WebUI Overriding](./webui_overriding.md) for dealing with modifying existing Chromium WebUI frontend (Lit / Polymer).
|
||||
Reference in New Issue
Block a user