Repository navigation
Commit fff07fb
test(app-shell): the new-hook target test waits for the roster's labels before reading them (objectui#11978) (#11986)
Fixes #11978
Clause-②: no
## What changed
One test, `ObjectHooksPanel.newHookTarget-11820.test.tsx`, "opens with
this package's objects first and every other object under the reach
warning": every read whose answer the object roster draws now waits for
that answer.
- The two "This package" checkbox reads (`Invoice (invoice)`, `Invoice
line (invoice_line)`) and the two "Other objects" checkbox reads (`User
(sys_user)`, `Employee (hr_employee)`) use `await
within(…).findByRole(…)` at the default timeout instead of a synchronous
`getByRole`.
- The "Other objects" group is found with `await screen.findByRole` too:
that group is drawn only once the roster answers.
- An empty-frontmatter changeset declares no release. The file sits
under `src/`, and `check-changeset-presence.mjs` has no carve-out for
tests.
The component is unchanged: `HookDefaultInspector`'s bare-name interim
state is deliberate (objectui#10585).
## Why it was red on timing alone
Two reads answer at their own pace.
- `ObjectHooksPanel` reads this package's objects in a mount effect and
hands them to the editor as `HookTargetScopeContext`. That draws the
"This package" group.
- `HookDefaultInspector` reads the object roster itself
(`useObjectOptions`), after New opens it. Until the roster answers,
`pickerOptions` holds only the selected object, labelled by its bare
name.
So "This package" can be on screen holding one checked checkbox named
`invoice` while "Other objects" is not drawn yet. The test awaited only
the group, then read the label synchronously. Which read answered first
decided the verdict: this is the red that took PR #11972 out of the
merge queue and turned PR #11973's `Test (shard 6/8)` red.
## Measurements, on this branch
**H1: the fix closes the window.** A throwaway, untracked copy of the
test (deleted afterwards) delays only the catalog `list('object')`
answer; the package-scoped read is not delayed. One copy of the test as
on `main` (`989b19082`), one of the test as changed here.
| copy | roster delay | runs | result |
|---|---|---|---|
| as on `main` (the lit control) | 120 ms | 3 | 3 failed, each at the
`Invoice (invoice)` read. The dump shows "This package" holding one
checked checkbox named `invoice`: the queue's signature. |
| as changed here | 120 ms | 3 | 3 passed |
| as changed here | 300 ms | 1 | passed |
| as changed here | 600 ms | 1 | passed |
| as changed here | 1500 ms | 1 | failed at the new
`findByRole('checkbox', { name: 'Invoice (invoice)' })`: the wait is
bounded by the default 1000 ms timeout. |
**H2: the test still tests its claim.** Two ablations of
`HookDefaultInspector.tsx` through objectstack's
`scripts/ablation-replace.mjs` in wrap mode. Each was restored to the
HEAD blob `3bb69262f529` with `git diff HEAD` empty, and a bash trap
re-checked the same hash.
- `invoice` moved from "This package" to "Other objects" (both
`scope.packageObjects.has(o.value)` partition reads replaced; anchor 2
to 0, blob changed). The test went red at the new `await
within(own).findByRole('checkbox', { name: 'Invoice (invoice)' })` after
its default timeout.
- The reach warning removed (the `hook-object-outside-reach` paragraph
deleted; anchor 1 to 0). Red at
`getByTestId('hook-object-outside-reach')`.
- The first attempt at the first ablation was a no-op. Its replacement
text contained the anchor, so the anchor count could not drop; the tool
refused with exit 1 before running the test, and restored. The reading
above is the retake, with a replacement that does not contain the
anchor.
**H3: the same pattern elsewhere.** Read only; no other test file is
touched. Two text scans over the 594 test files under
`packages/app-shell/src/views/studio-design/` and
`packages/app-shell/src/views/metadata-admin/`. Each has a lit positive
control: run on `main`, each hits this card's test; on this branch,
neither does.
1. An awaited container (`const X = await …findBy…`), then a synchronous
named read inside it: 15 hits in 10 files. None shares the race. The
names are static i18n strings drawn in the same commit as their
container (buttons, a tab, a placeholder). The one data-derived name,
`DocPreview.test.tsx` "keeps a group key no book declares visible", is
gated by an interposed `await waitFor(() =>
expect(picker).toHaveValue('retired_section'))`, which can pass only
once the books answered.
2. A synchronous read of a roster-shaped `Label (api_name)` name: 25
hits in 11 files. None shares the race. Most are synchronous renders
with no async read at all (static strings such as `Values (measures)`,
`role (deprecated)`). `HookDefaultInspector.reach-11820.test.tsx` awaits
`findByRole('checkbox', { name: 'Account (crm_account)' })`, the
roster's answer, before its synchronous reads.
`AccessExplainPanel.test.tsx` waits for the Object field to become a
select, which is drawn once the roster answered.
`ObjectFieldInspector.usePicklist-10202.test.tsx` awaits one served
option before reading its sibling from the same answer.
The scans' blind spot: a data-derived name in another form, read through
`screen.getBy…` after an unrelated await, is outside both.
## Gates, at `04a3cad2a`
- `pnpm exec vitest run --maxWorkers=2
packages/app-shell/src/views/studio-design/
packages/app-shell/src/views/metadata-admin/inspectors/` (verify lock):
`Test Files 296 passed (296)`, `Tests 2935 passed | 1 skipped (2936)`.
- `pnpm exec vitest run --maxWorkers=2 scripts/__tests__/` (verify
lock): `Test Files 179 passed | 2 skipped (181)`, `Tests 5452 passed | 2
skipped (5454)`.
- `pnpm --filter @object-ui/app-shell type-check` (verify lock; echoes
`@object-ui/app-shell@17.7.0 type-check`, which runs `tsc --noEmit &&
tsc -p tsconfig.test.json`): exit 0, after `turbo run build
--filter='@object-ui/app-shell^...'` built the dependency closure (28 of
28 tasks). `tsc -p tsconfig.test.json --listFilesOnly` lists this test
file. A first run before the closure was built could not resolve the
`@object-ui/*` declarations: NOT MEASURED, not a red.
- `node scripts/check-changeset-presence.mjs`: exit 0 (an empty
frontmatter, the explicit exemption). `check-changeset-no-major.mjs`,
`check-changeset-overwrite.mjs`, `pnpm check:changeset-claims`, `pnpm
check:pending-changeset-literals`: exit 0.
- `pnpm check:new-line-citations`: `0 new citation(s)`, exit 0. `pnpm
check:control-bytes`: exit 0.
- `pnpm check:vi-mock-specifiers`, `check:vi-mock-inherit`,
`check:vi-mock-override-shape`, `check:test-path-roots`: exit 0.
- ESLint, narrowed to the changed file, with an unchanged sibling test
as control: 2 files in the JSON result, 0 errors and 0 warnings each.
`--print-config` shows 119 rules active on the changed file, so it is in
ESLint's population. The config enables no type-aware linting, so this
diff cannot move another file's verdict. The repo-wide `pnpm lint` and
the full sharded `pnpm test` are CI's.
## Acceptance notes
- The sibling test of the same picker,
`HookDefaultInspector.reach-11820.test.tsx`, already waits the way this
change does: its mount helper awaits the roster-labelled checkbox before
any synchronous read.
- objectui#11913 waits on this card (`Blocked-by: #11978`). PR #11972
re-enters the queue once this lands.
Session: `https://claude.ai/code/session_01DBZ9bntPZ7VKyQNtJeNsgw`
---
_Generated by [Claude
Code](https://claude.ai/code/session_01DBZ9bntPZ7VKyQNtJeNsgw)_
Co-authored-by: Claude <noreply@anthropic.com>1 parent 4f4fc03 commit fff07fb
2 files changed
Lines changed: 16 additions & 5 deletions
File tree
- .changeset
- packages/app-shell/src/views/studio-design
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
Lines changed: 12 additions & 5 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
83 | 83 | | |
84 | 84 | | |
85 | 85 | | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
86 | 93 | | |
87 | | - | |
88 | | - | |
| 94 | + | |
| 95 | + | |
89 | 96 | | |
90 | | - | |
| 97 | + | |
91 | 98 | | |
92 | | - | |
93 | | - | |
| 99 | + | |
| 100 | + | |
94 | 101 | | |
95 | 102 | | |
96 | 103 | | |
| |||
0 commit comments