fix(test): Stabilize flaky test in useLoadingIndicator#3296
fix(test): Stabilize flaky test in useLoadingIndicator#3296BigUncle wants to merge 3 commits intogoogle-gemini:mainfrom
Conversation
Addresses flakiness in the 'should reflect values when Responding' test, which previously failed intermittently if the random phrase cycler selected the same phrase twice. This commit replaces the flawed assertion with a more robust check that always verifies the current loading phrase is a valid member of the WITTY_LOADING_PHRASES list, regardless of whether it changed. This ensures the test is deterministic and correctly validates the component's behavior. Incorporates feedback from AI code review.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @BigUncle, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request is focused on enhancing the reliability of our test suite by fixing a specific flaky unit test. The change ensures that a test involving randomized UI elements correctly validates their state, preventing intermittent failures and contributing to a more stable and trustworthy continuous integration pipeline.
Highlights
- Test Stability Improvement: I've addressed and resolved a flaky unit test located in
packages/cli/src/ui/hooks/useLoadingIndicator.test.ts. This test was intermittently failing because it incorrectly assumed that a randomly selected loading phrase would always be different after a set time interval. - Refined Test Logic: The core change involves modifying the test's assertion. Instead of asserting that the new loading phrase must be different from the previous one (which could fail if the same random phrase was picked twice), the test now robustly checks that the
currentLoadingPhraseis always a valid member of theWITTY_LOADING_PHRASESarray. This ensures correctness regardless of random selection. - Code Simplification: The test code has been simplified by removing an unnecessary
initialPhrasevariable and its associatednot.toBeassertion, resulting in a more concise and accurate test case.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request correctly identifies and fixes a source of flakiness in the useLoadingIndicator test by removing an assertion that could fail due to the random nature of phrase selection. While this resolves the instability, I've pointed out that it also reduces the test's effectiveness. I've suggested an alternative approach using mocking to create a deterministic and more robust test that can verify the phrase cycling logic without being flaky. This would be a stronger long-term solution.
|
Looks like all the changes were already merged in so closing out this PR. |
2 participants
TLDR
This pull request fixes a flaky (unstable) unit test in
useLoadingIndicator.test.ts. The test was failing intermittently because it incorrectly assumed a randomized loading phrase would always be different after a set interval.After incorporating code review feedback on an initial approach, this PR implements a more robust, single-line assertion. This new check correctly validates the loading phrase in all scenarios, ensuring the test is stable and reliable, which improves the health of our CI pipeline.
Dive Deeper
The root cause of the flakiness was the
should reflect values when Respondingtest case. It incorrectly asserted that the loading phrase must change after a set interval. However, since the phrases are chosen randomly from theWITTY_LOADING_PHRASESarray, there's a small but non-zero chance that the same phrase could be selected twice in a row, causing the test to fail.An initial fix attempt used an
if/elseblock. However, as pointed out during code review, that approach introduced a new logical flaw: it failed to validate the new phrase against the master list if the phrase was different.This PR implements the recommended, superior solution: a single assertion that always checks if the
currentLoadingPhraseis a valid member of theWITTY_LOADING_PHRASESarray. This is simpler, more correct, and fully resolves the flakiness while maintaining the test's integrity.Reviewer Test Plan
npm run preflightmultiple times.packages/cli/src/ui/hooks/useLoadingIndicator.test.ts, pass consistently every time. No further user-facing validation is needed as this is a test-only change.Testing Matrix
Linked issues / bugs
This PR addresses a flaky test discovered during the work on another branch. It does not close any specific existing issue but improves the overall health of the
mainbranch.