opensource-joe opened a new pull request, #3467:
URL: https://github.com/apache/apisix-dashboard/pull/3467
Please answer these questions before submitting a pull request, **or your PR
will get closed**.
**Why submit this pull request?**
- [x] Bugfix
**What changes will this PR take into?**
Fixes #3466. Test-only change, no `src/` code touched.
### The diagnosis changed once I could measure it
I originally suggested in #3466 that the helper should poll until its
content parses. That treats it as a race to wait out. Measuring it showed the
simpler story: the helper is reading sources that never hold the document.
`plugin_metadata.crud-all-fields.spec.ts` defined its own local
`getMonacoEditorValue` that tried the hidden `textarea` first and fell back to
joining `.view-line` elements. Neither is a source of editor content:
- the hidden `textarea` holds only a small buffer around the cursor, for IME
purposes, never the whole document
- `.view-line` elements are virtualised, so only painted lines exist
Sampling all three sources at the instant the Edit Plugin drawer becomes
visible, for a 219-character config:
```
t+0ms textarea= 0 "" viewLines= 1 "{" model=219
t+50ms textarea= 0 "" viewLines=11 "{ \"log_format\"…" model=219
t+150ms textarea= 0 "" viewLines=11 "{ \"log_format\"…" model=219
t+1000ms textarea= 0 "" viewLines=11 "{ \"log_format\"…" model=219
```
Two things fall out of that table:
1. **The textarea is empty at every offset**, so the first path never
contributed anything and the helper always fell through to the fallback.
2. **The fallback returns exactly `{`** until the editor paints, because the
first view-line of the JSON is the opening brace. That is precisely the
`Received string: "{"` in the failure.
The model is already complete at `t+0`. There was never a race worth waiting
out, just the wrong source being read. The paint window is roughly 50ms wide,
which is why this reproduces on a loaded machine and passes on an idle one.
### The fix
Add `uiGetMonacoEditorValue` next to the existing Monaco helpers and read
`window.__monacoEditor__.getModel().getValue()`.
That is not a new mechanism. It is the same source `uiFillMonacoEditor` and
`uiClearMonacoEditor` already **write** to, and the same one
`plugin-metadata.drawer-keeps-edits-on-failed-save.spec.ts` already **reads**.
The flaky spec was the outlier for having a private DOM-scraping helper at all.
It polls for non-empty content rather than reading once, because the drawer
mounts its editor asynchronously and `window.__monacoEditor__` can briefly
still point at a previously mounted instance after a new drawer opens.
`routes.empty-plugin-config.spec.ts` had the same defect, reading
`.view-lines` innerText. Its comment there ("Read Monaco editor content from
the visible lines (not the textarea)") shows the textarea problem was already
known, and the workaround reached for the other DOM source instead of the
model. Converted as well, so the pattern does not survive anywhere in `e2e/`.
### Verification
Against a live APISIX instance from `e2e/server`:
- the affected specs (`plugin_metadata.crud-all-fields`,
`plugin_metadata.crud-required-fields`, `plugin_metadata.list`,
`routes.empty-plugin-config`) pass **24/24** across 4 repeats
- `plugin_metadata.crud-all-fields` alone passes **10/10** with
`--repeat-each=10`
- `pnpm lint` clean at `--max-warnings=0`, `tsc -b` clean
**One honest caveat on the before/after.** I cannot show a clean
red-to-green locally, because on an idle machine the unpatched spec also passes
(10/10 when I tried). The failure needs the paint to lose a roughly 50ms
window, which happens under load. So the evidence that this is the right fix is
the sampling table above, which is deterministic and reproducible, rather than
a flip in pass rate. I also tried forcing it with CDP
`Emulation.setCPUThrottlingRate`; that does not work, because throttling slows
the test driver along with the renderer and the relative timing barely moves.
For the same reason, the earlier failure rates I quoted in #3466 (4 of 6 on
`master`) were measured while the machine was busy and should be read as "this
fails regularly under load", not as a stable rate.
**Related issues**
fixes #3466
**Checklist:**
- [x] Did you explain what problem does this PR solve? Or what new features
have been added?
- [x] Have you added corresponding test cases? (this is a test-only change;
it repairs existing coverage rather than adding new)
- [ ] Have you modified the corresponding document? (no document covers e2e
helpers)
- [x] Is this PR backward compatible?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
