feat: react testing guide - #830
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds a comprehensive React Testing Guide to the APL Console documentation to standardize how contributors write and review React tests (with emphasis on user-facing behavior, RTL query patterns, mocking boundaries, and async flows).
Changes:
- Added a new
docs/react-testing-guide.mdcovering recommended test goals, query priorities, mocking patterns, and common pitfalls. - Included concrete examples for RTK Query mutation mocking, React Router navigation mocking, and React Hook Form testing.
- Added a reviewer checklist and guidance on writing resilient, behavior-focused tests.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| SyntaxError: Cannot use import statement outside a module | ||
| ``` | ||
|
|
||
| did not mean the test contained invalid syntax. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
docs/react-testing-guide.md:1152
- This “Split by behaviour” snippet is tagged as TSX and uses
it("...");without callbacks. That’s invalid Jest syntax and could confuse readers; consider presenting these as plain text (or fullit("...", () => {})examples).
```tsx
it("populates the form from current settings");
it("trims node selectors before submitting");
it("removes empty node selectors");
it("submits null for an empty pull secret");
docs/react-testing-guide.md:326
- The “Good/Weak test names” snippets are marked as TSX and use
it("...");without a callback, which is invalid Jest syntax and easy to misread as copy/pastable code. Present these as plain text (or validit("...", () => {})examples) so the guide doesn’t model invalid patterns.
This issue also appears on line 1148 of the same file.
```tsx
it("disables saving while settings are being fetched");
it("preserves hidden Otomi settings when saving");
it("removes empty node selectors from the payload");
it("navigates to the settings overview after a successful save");
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
docs/react-testing-guide.md:464
- Sentence starts mid-thought and reads like a fragment after the code block; making it a standalone sentence improves readability (and matches the guide’s general-present tone).
did not mean the test contained invalid syntax.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
docs/react-testing-guide.md:464
- This sentence fragment starts with a lowercase verb and lacks a subject, which reads like a typo in the guide. It should be a complete sentence (e.g., starting with “This error …”).
did not mean the test contained invalid syntax.
docs/react-testing-guide.md:60
- These fenced blocks are marked as TypeScript (```ts) but the content is just a list of field names with semicolons. That reads like misleading TS and the syntax highlighting is wrong. Consider switching to a plain-text fence and omitting the semicolons, or show a real TS type/interface snippet.
```ts
version;
hasExternalDNS;
hasExternalIDP;
globalPullSecret;
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
docs/react-testing-guide.md:464
- Sentence starts mid-thought and with a lowercase letter, which reads like a fragment after the preceding code block. Consider rewriting it as a complete sentence with a subject.
did not mean the test contained invalid syntax.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
docs/react-testing-guide.md:464
- This sentence starts mid-thought and reads like it’s referring to a past event. If the intent is to explain the meaning of the error generally, rephrase it as a complete sentence (and capitalize it).
did not mean the test contained invalid syntax.
docs/react-testing-guide.md:3
- The guide introduces a new
npm run coveragescript in package.json, but the doc doesn’t currently mention how to run tests/coverage locally. Adding a short “Running tests locally” section near the top would make the guide immediately actionable.
# React Testing Guide for the APL Console
## 1. Why we test React code
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
docs/react-testing-guide.md:466
- The sentence after the error snippet is a fragment starting with “did not mean…”. Consider rewriting it as a complete sentence (e.g., “This error does not mean …”) for clarity.
did not mean the test contained invalid syntax.
The import chain was:
</details>
| The APL Console contains several areas where regressions are easy to introduce: | ||
|
|
||
| - forms generated from API data | ||
| - RTK Query mutations and refetching |
There was a problem hiding this comment.
RTK are the queries used to fetch data from the api like
useGetAllAplWorkloadsQuery() useGetTeamAplWorkloadsQuery() useGetAplNamespaceSealedSecretsQuery() useGetAllAplBuildsQuery()
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (5)
docs/react-testing-guide.md:771
- These fenced blocks (```tsx) contain only identifiers (
isLoading;, `isFetching;`) which aren’t valid TSX. Using inline code makes the guidance clearer and avoids suggesting this is runnable code.
For example:
```tsx
isLoading;
usually represents the initial request.
isFetching;**docs/react-testing-guide.md:464**
* This sentence starts mid-thought and with a lowercase letter. Consider rephrasing to a complete sentence so the guidance reads cleanly in the docs.
did not mean the test contained invalid syntax.
**docs/react-testing-guide.md:57**
* Same issue here: this is a list of fields, not valid TypeScript. Using a plain text-style block (and dropping semicolons) makes the example clearer and avoids misleading syntax highlighting.
adminPassword;
isMultitenant;
isPreInstalled;
aiEnabled;docs/react-testing-guide.md:49
- These fenced blocks are tagged as TypeScript (```ts) but the contents aren’t valid TS statements (they’re property name lists). Using a plain text-style block avoids implying this is copy/pasteable code and improves readability.
This issue also appears on line 53 of the same file.
```ts
version;
hasExternalDNS;
hasExternalIDP;
globalPullSecret;
docs/react-testing-guide.md:175
- These blocks are labeled TSX but the contents are just identifier lists; they aren’t valid TSX/JS. Consider using plain text-style blocks (or inline code) to avoid implying this is executable code.
This issue also appears on line 761 of the same file.
```tsx
getByRole;
getByLabelText;
getByText;
getByPlaceholderText;
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (2)
docs/react-testing-guide.md:59
- Same issue here: inside a ```ts block these are not valid TS; representing them as an API type (optionals, nesting, etc.) will be clearer and copy/pasteable.
adminPassword;
isMultitenant;
isPreInstalled;
aiEnabled;
useORCS;
git;
docs/react-testing-guide.md:49
- Inside a ```ts block, these lines are not valid TypeScript (they’re expression statements like
version;). Since this is a guide, it’s clearer to show these as a type/interface (or object shape) so readers don’t copy/paste invalid code.
This issue also appears on line 54 of the same file.
version;
hasExternalDNS;
hasExternalIDP;
globalPullSecret;
nodeSelector;
</details>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (3)
docs/react-testing-guide.md:53
- This fenced block is tagged as
ts, but the contents are not valid TypeScript (just field names). Consider using atextfence (or writing a valid TS type/interface) to avoid misleading syntax highlighting.
```ts
docs/react-testing-guide.md:464
- This line is a sentence fragment and reads as though it’s missing a subject (and it’s phrased like a one-time past event). Reword to a complete, general statement.
did not mean the test contained invalid syntax.
docs/react-testing-guide.md:43
- The fenced block is tagged as
ts, but the contents are just field names and not valid TypeScript. This is likely to confuse readers and produces misleading syntax highlighting; use atextfence (or a valid TS type snippet) instead.
This issue also appears on line 53 of the same file.
```ts
There was a problem hiding this comment.
🟢 Approval recommended
Changes are low-risk (docs + a script alias) and no functional/runtime behavior is modified.
Review details
Suppressed comments (1)
docs/react-testing-guide.md:464
- The sentence after the error snippet uses past tense and reads like a historical incident; since this section describes a general rule, it should be in present tense (and include the implied subject from the preceding line).
did not mean the test contained invalid syntax.
- Files reviewed: 2/2 changed files
- Comments generated: 0 new
- Review effort level: Lite
Considerations