Repository navigation
🧪 fix: Wait Out the Langfuse Load-Time Verification in Its Tests and Bring the Live Model Specs Journey Up to Date - #204
Merged
Conversation
…Bring the Live Model Specs Journey Up to Date
LangfuseRenderer verifies a saved connection once its status renders, and
while that request is in flight every control is disabled and the primary
button reads "Checking" instead of "Save & enable". "preserves the stored
secret when only the public key is edited" and "enables a disabled connection
when credential edits are saved with Save & enable" clicked Edit as soon as
the stored connection showed, before that verification had run or settled,
so under load the click landed inside the in-flight window and the Save &
enable lookup failed. renderVerified() now renders a saved connection and
waits for the Verified status, which renders in the same pass that releases
the controls; every test that acts on a saved connection uses it, with its
assertions unchanged. The default verification mock answers on a later task,
as a request does, so the window is always there and a test that acts inside
it fails on every run instead of rarely. A new test holds the verification
open and checks that Edit and Disable stay disabled until it settles.
e2e/specs.spec.ts expected a new spec to open on Endpoint. That held only
while a new spec showed its Model group first; the editor then went back to
one group order for new and saved specs (Basics, then Model), so a new card
focuses its Label, and the journey was never updated. The same journey also
expected the card summary to read "openAI · gpt-4o-mini", but spec rows name
the endpoint ("OpenAI") since the change that made them do so. addSpec now
expects Label focused and fills Label, Endpoint and Model in that order, and
the summary assertion expects "OpenAI · gpt-4o-mini". A unit test pins the
new spec's group order and its Label focus, so CI catches the next change.
Contributor
Author
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
Codex Review: Didn't find any major issues. Bravo. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
4 tasks done
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull Request Template
Summary
This PR fixes two unreliable tests on
main. In both cases the test was wrong and the panel was right, so the PR changes only test files:LangfuseRenderer.test.tsx,ModelSpecsRenderer.test.tsxande2e/specs.spec.ts.LangfuseRenderer.test.tsxraced the verification the renderer starts on load."preserves the stored secret when only the public key is edited" failed once in CI with
Unable to find an accessible element with the role "button" and name "com_config_langfuse_save_and_enable", then passed on re-run. Here is the async step it raced:useEffectinLangfuseRendererverifies it once the status has rendered (testMutation.mutate). While that request is in flight (testMutation.isPending), every control is disabled and the primary button reads "Checking" instead of "Save & enable".getByRolefor Save & enable failed. When the effect had already committed its in-flight render, the Edit button was disabled, and the click did nothing.mainfailed 2 of 48 runs with the CI error. The failure output shows the public key edit open and the Save button readingcom_config_langfuse_checking, disabled.The other tests in the file:
mockTestto have been called once. That is already true while the request is in flight, so these two passed only because the in-flight render usually had not committed when they clicked.The fix:
renderVerified(status)renders a saved connection and waits for the Verified status. Verified renders in the same pass in which the request is no longer pending, so the controls are enabled by then. It also asserts that exactly one verification ran. Every test that acts on a saved connection uses it (six tests), and their assertions are unchanged.setTimeout(0)), as a real request does. The in-flight window is therefore always there, and a test that acts inside it fails on every run, not on one run in twenty under load. With this mock alone and the old test bodies, exactly the two tests that flaked fail, 5 runs out of 5.configuration/sections/that runs queries or mutations, so no other section test races a request. The ones that do wait for something (mcp/Servers.test.tsx,mcp/Create.test.tsxandfiles/FileConfigRenderer.test.tsx) already await their dialogs and comboboxes withfindByRole, and a stress run of all of them found no flakes (see Testing).e2e/specs.spec.ts: the live Model specs journey expected an editor order the panel dropped."a new spec asks for Endpoint and Model up front and saves at the end of the list" failed live even when it ran alone on a clean database.
toBeFocusedon the new card's Endpoint select got "inactive", and the serial describe then skipped the remaining 8 journeys. The new card had focused its Label input. This is test drift, not a panel bug:ObjectEntryCard). With Basics first, that field is Label. The currentEditor.tsxcomment says the same: "Basics and Model (with the required name, label, endpoint and model) open first". Both groups are open on a new spec, with Endpoint and Model marked required.With the focus expectation fixed, the same test failed on a second stale assertion that the first failure had been hiding. Since "spec rows name the endpoint", which landed hours after the journey was written, the card summary names the endpoint by its display name:
e2e-spec · OpenAI · gpt-4o-mini. The test still expected the raw key,openAI · gpt-4o-mini.The fix:
addSpecexpects Label to be focused, then fills Label, Endpoint and Model in the order the card shows them. The summary assertion expectsOpenAI · gpt-4o-mini.ModelSpecsRenderer.test.tsxgains "opens a new spec on its Label, with Basics before Model as on a saved spec", which runs in CI. With the editor's groups swapped back to Model first, it fails. The stateful harness from the reorder test moved to module level, so both tests share it. The rename test and the new test share a helper that reads the group order.Not in this PR
revertListonly runs once every assertion has passed. A later run then derivese2e-spec-2and fails. That happened while I was debugging this PR, never in the final runs.Change Type
Testing
Unit tests,
LangfuseRenderer.test.tsxmain, 20 sequential runsmain, 48 runs in 8 concurrent loopsmain's file in the same 60 runs, in 12 concurrent loops plus 16 busy-loop CPU burnersmain's copy: 4 failed (the same two tests, twice each)configuration/sections/(715 tests) on this branch's final commit, 12 runs in 4 concurrent loopsUnit tests,
ModelSpecsRenderer.test.tsx: 32 passed. WithEditor.tsxchanged to render the Model group before Basics (not committed), the new test fails on the group order.Live journeys,
e2e/specs.spec.tsAll runs used
E2E_LIVE=1against a local LibreChat backend. A clean database here means the adminconfigscollection was emptied and the backend restarted: the backend caches the merged config, so emptying the collection alone still served the previous run's saved spec.maintoBeFocused, got "inactive"; 8 journeys did not run. The focused element was the new card's Label input.toContainText('openAI · gpt-4o-mini'); the card reade2e-spec · OpenAI · gpt-4o-miniAfter the runs, Base and USER held no overrides. Against the mock backend, the spec runs its setup and skips its 10 live journeys, as before.
Test Configuration:
npx tsc --noEmit,npx eslint src/ --max-warnings 0, Prettier on the changed files andbun run test:lint-rules(19 passed) are clean.npx vitest run --maxWorkers=3: 162 files, 3514 tests passed.bun run buildsucceeds.Checklist