HOMS-523 Fix flaky feature specs

[kompl]

Stabilize the feature suite before bumping major gem versions, so that the effect of the bump can be measured against a green baseline.

• delete_user_spec: wait for the sign in to complete before visiting /users, otherwise the navigation cancels the in-flight POST and the test lands on the login page (42% of runs). Replace the legacy current_path assertion with the retrying have_current_path (8%).
• expect_widget_presence: all() ignores :wait unless a count expectation is given, so it returned an empty list before the widget had rendered (17% of runs in new_order_spec). Use have_css, which retries.
• wait_for_ajax: poll with an interval instead of a busy loop, which flooded the CDP channel with evaluate_script calls while the page was re-rendering.
• Drop Capybara.automatic_reload = false: it disables the re-lookup of stale nodes and is the source of the "stale element reference" and "Node with given id does not belong to the document" errors.

[TimAle]

LGTM but most of these comments should not stay

[kompl]

Why the non-obvious bits look the way they do. This is context for reviewing the change rather than for reading the code, so it lives here instead of in the tree.

[kompl]

Yarn is provisioned by corepack, which resolves the version from the packageManager field of package.json. yarn set version pins it a second time, next to packageManager, and downloads the release from repo.yarnpkg.com rather than from the registry.

[kompl]

Puma 8 binds to :: instead of 0.0.0.0 whenever a non-loopback IPv6 interface is up, so what the server listens on would otherwise depend on the host it boots at. Pinned, as config/unicorn.rb does for production.

This file is only read by rails s: Capybara builds Puma::Server itself and passes its own host and port (hence the random port in the test log), and production runs on unicorn.

[kompl]

The progress formatter prints every example as a dot on one unterminated line, and CI renders logs line by line — so the whole run stayed invisible until the newline that ended it, then arrived all at once. One line per example keeps the log moving and names the spec a stuck run is sitting on.

[kompl]

Keep at least one thread alive: with a minimum of zero the pool trims its last thread while the server is waiting for one to free up, and with queue_requests off there is no reactor to hold the request meanwhile — the server then waits forever for a worker that will never be spawned (puma#876).

A hung server is a good match for the symptom the ticket opens with: a pile of tests failing at once with MultipleExceptionError and nothing in the log, because what breaks is the transport rather than any one test. config/puma.rb runs production on the same 1:1 pool.

[kompl]

all() without a count expectation ignores :wait and returns immediately, so the widget's children may not have rendered yet — have_css retries. That is what produced the expected [].empty? to be falsey this replaces.

The explicit wait: 5 is gone too: it was half the suite's own wait time, while the tick sleeps that used to cover this allowed more.

[kompl]

Why the mock gap gets a name of its own — context for the review, not for reading the code.

[kompl]

Why the mock fallback and the camunda-mock trace look the way they do — context for reviewing, not for reading the code.

[kompl]

This trace is what located the flaky order specs, so it is worth keeping.

A gap in a mock file was indistinguishable from a Camunda outage: fetch_response missed, do_request rescued it into a RemoteError, HBW::APIController rendered that as a 504, and the widget reported the 504 as "Camunda service is unavailable" — the same growl a real outage produces, in an environment where no Camunda exists to be down.

Logging every request with HIT/MISS makes a failing run replayable: the miss is read against the hits before it, and against the stub that was connected. In the red run (29726222780) that turned a vague growl into one exact line — all 27 misses were the same request.

[kompl]

The widget polls a handful of endpoints on every page it renders, and a feature's mock file usually only covers what its own test exercises. Every uncovered poll became the 504 growl described on common/yml_api.rb — 44 of them in one suite run, on specs that passed anyway and so never pointed at the cause.

Merging a global mock under the test mock closes that class at the source instead of per file. The test mock's entries are placed first, so a test that does mock an endpoint keeps full control of it and the global mock only fills what is left.

Result: process-definition misses went 44 → 27 → 0 across 29726222780 and 29729795637, and Create new order success vacation request — which had been failing ~17% of runs — passes.

[kompl]

Endpoints the widget polls on every page but that a feature's own mock often omits, because its test does not exercise them. set_camunda_api_mock_file merges this file under the test mock.

The two entries are deliberately the same endpoint: the widget queries process-definition bare (the full list) and filtered by BP user for the start button that /widget/buttons renders. Only the bare form was covered at first, so the filtered one still missed on every widget page — 27 times in 29726222780 — which is why both are here.

An empty list means "nothing to start", so no start button renders. That is right for any test that does not drive process starting; one that does supplies its own entry, which wins.

[kompl]

A miss only means something against the mock file that was connected when it happened, so the stub line has to land in the same log as the HIT/MISS trace. With both, server-log-homs (the artifact the homs-ci change attaches) is enough to reconstruct a failing run offline — which stub, then which requests it did and did not cover.

The process-keys suffix is for the two-backend stub in two_bpm_backends_spec, where two mock files back one :api and a miss on one of them is expected fan-out rather than a gap.

[kompl]

Why the sign-in guard and the two mock-layer changes look the way they do — context for reviewing, not for reading the code.

[kompl]

signin occasionally left the suite sitting on the login page. That is the failure that made the task specs flaky — expected not to find text "Sign in" in "Hydra OMS\nSign in\nRemember me", raised from the shared before hook rather than from anything the example itself did.

The server log of run 29807725008 pins the mechanism. The failing example issued GET /users/sign_in at 07:08:14 and no POST ever followed before it gave up at 07:08:25 — the form was never submitted. The page dump captured at failure shows both inputs empty and Application.messenger.show([]): the pristine login page, not a rejected-credentials re-render, which Devise would have given a flash. So fill_in had reported success against a form that was not yet interactive — the input exists in the static HTML and is findable the instant the DOM parses, but the typed value never stuck, and the click that followed submitted nothing. The two examples that ran straight after signed in cleanly, so this is an intermittent readiness race, not a broken form.

Confirming the value actually stuck is what closes it: by the time the guard has re-driven the form, it is interactive, so the submit fires.

A first attempt asserted the outcome instead — expect(page).to have_no_button('Sign in'). That broke the three sign_in_spec scenarios which sign in with invalid credentials and expect to stay on the login page. This helper serves both paths, which is why the guard deliberately asserts nothing about whether the sign-in succeeded.

[kompl]

Array() reads like the natural "wrap a lone entry, pass a list through" idiom, but Array(Hash) calls to_a. A single entry written as a mapping would become [["params", ""], ["response", []]], and fetch_response's find { |el| el['params'] == query } would then index an Array with a String and raise TypeError.

Every mock file currently uses lists, so nothing triggers this today — the change removes the mine rather than fixing a live bug. Verified behaviour-preserving: merging the global mock against each of the 48 test mocks gives identical results under the old and the new implementation (48 identical, 0 differing).

Raised by glm-5.2 in cross-review.

[kompl]

A mock miss used to surface as undefined method 'fetch' for nil — entry was nil and entry.fetch('response') blew up on it. do_request rescues StandardError and rebuilds it as RemoteError.new(args[0], e.message, e.backtrace), so that message is exactly what reaches the log and the 504. Raising here makes it name the miss instead: no camunda mock entry for GET process-definition params="...".

Behaviour is unchanged — KeyError is still a StandardError, so it takes the same RemoteError → 504 path, and nothing rescues KeyError specifically.

Raised by kimi-k2.7 in cross-review.