meta(refactor-tasks): Add the no-custom-render-helper convention

.agents/skills/react-testing/SKILL.md

@@ -120,6 +120,29 @@ const project = ProjectFixture(partialProject)
 
 ```
 
+### Render a component, not a `renderFoo()` helper
+
+A local helper hides the JSX from every test that calls it, and `rerender` takes
+an element, so the call site can no longer control what renders.
+
+```tsx
+// ❌ Don't wrap render() in a helper
+function renderComponent(props: Props = {}) {
+  return render(<Widget {...props}>hello</Widget>);
+}
+
+// ✅ Put the fixed parts in a component
+function ExampleWidget(props: Props) {
+  return <Widget {...props}>hello</Widget>;
+}
+render(<ExampleWidget isLoading />);
+```
+
+Declare it at module scope so `rerender` keeps the same component type, and keep
+`render()` options at the call site. If the helper adds nothing over the
+component's own props, drop it and call `render(<Widget />)` directly. Helpers
+that only register mocks, or that take the element as a parameter, are fine.
+
 ### Use `screen` instead of destructuring
 
 ```tsx

.sentry-refactor-tasks/conventions/no-custom-render-helper.yaml

@@ -0,0 +1,161 @@
+name: no-custom-render-helper
+severity: warning
+tags: [testing, react, jest]
+
+why: |
+  Test files often grow a local `renderFoo()` helper that only forwards its
+  arguments into the shared `render()` / `renderHookWithProviders()` helper from
+  `sentry-test/reactTestingLibrary`. Every file invents its own name and its own
+  argument shape, so a reader must first go read the helper before any `it()`
+  block makes sense, and the standard RTL return value (`rerender`, `router`,
+  `unmount`) sometimes gets dropped or renamed on the way out.
+
+  When a test needs fixed props, wrapper providers or a bit of glue, put that in
+  a component instead of in a function. A small `ExampleFoo` component keeps
+  every call site in the shape the rest of the codebase already uses —
+  `render(<ExampleFoo />)` — so the JSX is visible in the test, extra props can
+  be passed per test, and `rerender(<ExampleFoo isLoading />)` works the way RTL
+  documents it.
+
+  See: .agents/skills/react-testing/SKILL.md
+
+detect: |
+  In a test file (`*.spec.ts` / `*.spec.tsx`), look for a locally declared
+  function whose only job is to call the shared render helper:
+
+    - A `function renderX(...)` declaration, or a `const renderX = (...) => ...`
+      arrow, declared in the test file or inside a `describe()` block.
+    - Its body returns (or directly calls) `render(...)`, `renderGlobalModal(...)`
+      or `renderHookWithProviders(...)` imported from
+      `sentry-test/reactTestingLibrary`.
+    - The JSX / hook callback it renders is essentially fixed: the helper's
+      parameters are spread into props, used to pick between a few literal
+      prop values, or not used at all.
+
+  Flag the helper declaration itself.
+
+  Do NOT flag:
+    - Helpers that do real test setup beyond rendering — registering
+      `MockApiClient.addMockResponse()` mocks, seeding stores, building fixtures,
+      advancing timers — and never call `render()` (e.g. `renderMockRequests()`).
+    - Helpers that return extra handles the test needs alongside the render
+      result, such as a `jest.fn()` spy installed through a context provider
+      (`return {tracking, ...render(ui, {additionalWrapper})}`).
+    - Helpers that take the element to render as a parameter and only supply
+      providers or options (`renderWithX(ui)`), since there is no fixed JSX to
+      move into a component.
+    - Render-prop callbacks and test components that return JSX without calling
+      `render()` at all.
+    - `render` / `renderHookWithProviders` themselves in
+      `tests/js/sentry-test/reactTestingLibrary.tsx`, and any other shared helper
+      under `tests/js/`.
+
+fix: |
+  Replace the helper function with a component, then call the shared `render()`
+  at each call site.
+
+  1. Name the component `Example<Thing>` — `ExampleLoadingContainer`,
+     `ExampleWidgetModal`. (Older tests in the repo use `Test<Thing>`; either
+     name is fine, be consistent inside a file.)
+  2. Give it the props the helper took as arguments, so each test can vary only
+     what it cares about.
+  3. Move provider wrappers into the component's JSX; move `render()` options
+     (`organization`, `initialRouterConfig`, `additionalWrapper`) to the call
+     sites that need them.
+  4. Delete the helper and call `render(<ExampleThing ... />)` in each test.
+
+  Example before:
+    function renderComponent(props: LoadingContainerProps = {}) {
+      return render(
+        <LoadingContainer {...props}>
+          <div>hello!</div>
+        </LoadingContainer>
+      );
+    }
+
+    it('handles loading', () => {
+      const {rerender} = renderComponent({isLoading: true});
+    });
+
+  Example after:
+    function ExampleLoadingContainer(props: LoadingContainerProps) {
+      return (
+        <LoadingContainer {...props}>
+          <div>hello!</div>
+        </LoadingContainer>
+      );
+    }
+
+    it('handles loading', () => {
+      const {rerender} = render(<ExampleLoadingContainer isLoading />);
+      rerender(<ExampleLoadingContainer />);
+    });
+
+  For a hook helper, the same move applies to `renderHookWithProviders`:
+
+    // Before
+    function renderProviders() {
+      return renderHookWithProviders(() => useScmMessagingProviders(), {organization});
+    }
+    // After — inline the hook call, keep the options at the call site
+    const {result} = renderHookWithProviders(() => useScmMessagingProviders(), {
+      organization,
+    });
+
+  Gotchas:
+    1. Do not re-export the render result through a renamed object; tests expect
+       the standard `{rerender, unmount, router}` shape from `render()`.
+    2. A helper that also returns a spy or a mock handle is doing two jobs —
+       move the JSX into the component and leave the spy setup in the test, or
+       leave the helper alone if the spy must be installed through a provider.
+    3. Components declared inside `describe()` re-create their identity on every
+       call; declare `ExampleFoo` at module scope so `rerender` keeps the same
+       component type and preserves state.
+    4. Helpers that are `async` usually await something after rendering (a
+       `findBy` query, a router navigation) — that awaited part belongs in the
+       test body, not in the component.
+
+examples:
+  bad:
+    - |
+      function renderComponent(props: LoadingContainerProps = {}) {
+        return render(
+          <LoadingContainer {...props}>
+            <div>hello!</div>
+          </LoadingContainer>
+        );
+      }
+    - |
+      const renderAvatars = ({users, teams}: Props) =>
+        render(<AvatarList users={users} teams={teams} />);
+    - |
+      function renderProviders() {
+        return renderHookWithProviders(() => useScmMessagingProviders(), {organization});
+      }
+  good:
+    - |
+      function ExampleLoadingContainer(props: LoadingContainerProps) {
+        return (
+          <LoadingContainer {...props}>
+            <div>hello!</div>
+          </LoadingContainer>
+        );
+      }
+      // in the test: render(<ExampleLoadingContainer isLoading />);
+    - |
+      // Setup helper that registers mocks and never renders
+      function mockProjectRequests({firstIssue}: {firstIssue?: string} = {}) {
+        MockApiClient.addMockResponse({url: '/projects/org-slug/project-slug/', body: {}});
+      }
+    - |
+      // Wrapper that installs a spy through a provider and takes the element
+      function renderWithTracking(ui: React.ReactElement) {
+        const tracking = jest.fn();
+        return {tracking, ...render(ui, {additionalWrapper: TrackingWrapper})};
+      }
+
+include:
+  - 'static/**/*.spec.tsx'
+  - 'static/**/*.spec.ts'
+
+prefilter: "grep -rlE '^[[:space:]]*(export )?(async )?function render[A-Z]|^[[:space:]]*(const|let) render[A-Z][A-Za-z0-9]*[[:space:]]*=' --include='*.spec.tsx' --include='*.spec.ts' {repo_path}/static/"