# Close documentation gaps in `wait_for_requests` removal guidance and `.ai/principles/manifest.yml` coverage Investigation of !246008 against the SSOT documentation listed in [`.ai/principles/manifest.yml`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/.ai/principles/manifest.yml). The MR's substitutions encode roughly a dozen synchronization techniques. Most were undocumented, and one page taught the banned pattern. `RSpec/AvoidWaitForRequests` is enabled with **530 remaining file exclusions** in `.rubocop_todo/rspec/avoid_wait_for_requests.yml`. <details> <summary>Original investigation prompt</summary> > Investigate https://gitlab.com/gitlab-org/gitlab/-/merge_requests/246008 and see if there are any gaps in the documentation listed as sources for the distillation process in .ai/principles/manifest.yml </details> ## Investigation results ### Already covered `doc/development/testing_guide/best_practices.md` (a `testing-rspec` manifest source) covers: - Avoiding absence checks before a positive page-load assertion. - Stable end-state assertions rather than transient controls. - `wait_for` as a last resort for browser side effects. - Waiting matchers instead of point-in-time `.value`, `.text`, and `.count` reads. `doc/development/testing_guide/frontend_testing.md` also says to assert a visible element before backend attributes. ### Gaps | ID | Finding | |---|---| | A1 | `doc/development/fe_guide/accessibility/feature_tests.md:88` taught `wait_for_requests # ensures page is fully loaded` in the canonical axe example. This contradicted the cop and the file was not a `frontend-a11y` manifest source. | | B1-B3 | No SSOT page named `RSpec/AvoidWaitForRequests`, said not to add todo exclusions or inline disables, documented `wait_for_all_requests`, or fully explained the poll-between-requests race. | | C1 | `.visible?` after `find` is vacuous, but the waiting-matcher guidance did not cover it. | | C2 | `evaluate_script` and `execute_script` are point-in-time reads. The MR gates `scrollTop` with `wait_for` and batches related reads atomically. This was undocumented. | | C3 | `wait_for` tuning (`max_wait_time`, `polling_interval`) was undocumented. !246008 needed slower polling to avoid excess database load. | | C4 | There was no guidance for narrowly scoped production readiness signals used by tests (`data-right-sidebar-initialized`) or for distinguishing them from the timer-based `page-initialised` class. | | C5 | Shared-example readiness gates were undocumented. Rapid Diffs documents `diff-file-mounted`, but its full architecture page is not an appropriate testing-principle source. | | C6 | The guidance did not cover waiting for an animated container before interacting with a transient control such as GlModal's disabled close button. | | C7-C9 | The waiting-matcher guidance did not cover `page.current_url`, `exact_text:`, or `have_gitlab_http_status` on `Capybara::Session` (`status_code`), the latter causing a false-positive Duo comment on !246008. | | D1 | Relevant SSOT files were absent from the manifest: `accessibility/feature_tests.md` and `testing_guide/unhealthy_tests.md`. | | D2 | Neither testing principle covered `spec/support/**`, so load-bearing shared-example and helper changes received no testing guidance. | | D3 | Test-only production changes are invisible to testing principles. This is out of scope because glob-only `file_filters` cannot target those changes narrowly. | | D4 | Synchronization guidance was scattered across multiple testing-guide pages, unlike QA's dedicated waits page. | ## Decisions - **D3 is out of scope.** Adding `app/assets/javascripts/**/*.js` to a testing principle's `file_filters` would apply testing guidance to every JavaScript change. The manifest supports glob-only routing, so there is no sufficiently narrow expression for test-readiness hooks in production code. - **SSOT first.** Put reusable engineering guidance in `doc/development/`, not a baseline. Baselines remain for temporary, agent-only rules that do not belong in public development documentation. - **Rapid Diffs is not a testing-hierarchy source.** Only about 7% of the 604-line page concerns testing, and that content is how-to-write guidance which `testing-frontend-testing-hierarchy` delegates to `testing-jest`. The `diff-file-mounted` readiness gate is documented through `best_practices.md` instead. Follow-up: #608167 proposes extracting the focused Rapid Diffs guidelines into a separate page before deciding its distillation target. - **Two merge requests.** Documentation and `.ai/` manifest configuration are separate along CODEOWNERS boundaries. The manifest MR can merge first because its newly declared source files already exist on `master`. - **Canonical section placement.** `Never use wait_for_requests or wait_for_all_requests` appears immediately before the previous Section 617. The cop links directly to that section, then readers flow into detailed replacement techniques. - **Readiness-signal guidance location.** Production-code readiness signals are documented in `doc/development/fe_guide/style/javascript.md`, with a cross-link to the testing guide. This is a production-code design rule even though its immediate consumer is a feature spec. ## Implementation status ### SSOT documentation and cop link [!248680](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/248680) documents reliable feature-spec synchronization and updates the cop message: - Replaces the accessibility-guide `wait_for_requests` example with a positive readiness matcher. - Adds the canonical prohibition, its race explanation, and purpose-specific alternatives. - Documents stable modal readiness, `wait_for` tuning, point-in-time reads, scripts, shared-example readiness gates, and `have_gitlab_http_status` support for Capybara sessions. - Adds readiness-signal guidance to the JavaScript style guide. - Links the frontend-testing assertions primer to the canonical rule. - Repoints `RSpec/AvoidWaitForRequests::MESSAGE` and its expectations to the canonical docs anchor. - Applies the documentation style guide after Duo review; all three Duo threads are resolved. ### Distillation manifest coverage [!248682](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/248682) expands manifest coverage without hand-editing generated principles: - Adds `doc/development/fe_guide/accessibility/feature_tests.md` to `frontend-a11y.sources`. - Adds `doc/development/testing_guide/unhealthy_tests.md` to `testing-rspec.sources`. - Adds `spec/support/**/*.rb` and `ee/spec/support/**/*.rb` to `testing-rspec.file_filters`. - Validates the manifest and all declared source paths. The changes invalidate checksums. After merge, the scheduled distillation pipeline should regenerate `frontend-a11y` and `testing-rspec` and open per-team merge requests. Verify the generated output before closing this issue. ## References - !246008 - the MR investigated - &22879 - Remove all uses of `wait_for_requests`, `wait_for_all_requests` in our RSpec suite - #525094 - Investigate whether we could remove Capybara wait times - !234146 - added `RSpec/AvoidWaitForRequests` - #608167 - Extract Rapid Diffs guidelines into a focused documentation page - [`rubocop/cop/rspec/avoid_wait_for_requests.rb`](https://gitlab.com/gitlab-org/gitlab/-/blob/master/rubocop/cop/rspec/avoid_wait_for_requests.rb)