test(integration): skip flaky 'should exit gracefully on second Ctrl+C' test#23731
test(integration): skip flaky 'should exit gracefully on second Ctrl+C' test#23731
Conversation
…C' test This test is currently flaky on macOS and is causing E2E test failures in CI.
Summary of ChangesHello, 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 aims to improve the stability of the continuous integration pipeline by temporarily disabling a specific integration test. The test, which simulates multiple Ctrl+C signals, was found to be unreliable on macOS, leading to intermittent build failures. By skipping this test, the PR ensures that the main branch remains unblocked, allowing for continued development while a permanent fix is being investigated. Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request temporarily disables a flaky integration test (ctrl-c-exit.test.ts) by marking it as it.skip. The review suggests creating a follow-up issue to track the fix for the flakiness and adding a TODO comment referencing the issue number, in line with development conventions for managing technical debt and maintaining code quality.
| afterEach(async () => await rig.cleanup()); | ||
|
|
||
| it('should exit gracefully on second Ctrl+C', async () => { | ||
| it.skip('should exit gracefully on second Ctrl+C', async () => { |
There was a problem hiding this comment.
While skipping a flaky test is a pragmatic short-term solution to unblock CI, it introduces technical debt and a gap in test coverage. To ensure this is addressed, please create a follow-up issue to track fixing the flakiness and add a TODO comment referencing the issue number. This aligns with development conventions to track work and maintain code quality, as mentioned in the style guide (lines 64-65).
// TODO(#<issue-number>): This test is flaky on macOS and has been temporarily disabled. It should be re-enabled once the underlying issue is fixed.
it.skip('should exit gracefully on second Ctrl+C', async () => {References
- Development conventions to track work and maintain code quality. (link)
|
Size Change: -4 B (0%) Total Size: 26.3 MB
ℹ️ View Unchanged
|
Summary
This PR skips the
should exit gracefully on second Ctrl+Cintegration test inctrl-c-exit.test.ts. This test is currently flaky on macOS and is causing E2E test failures in CI.Details
The test uses multiple
ctrl-csignals which are proving to be unreliable specifically on the macOS environment in CI runs. It is disabled to unblock the main branch while a more robust solution is developed. Tagging @SandyTao520 for visibility as they were the last person to touch this file/test.Related Issues
Related to #23733
How to Validate
Run the integration tests to confirm the test is skipped:
npm test -w @google/gemini-cli-core -- integration-tests/ctrl-c-exit.test.tsPre-Merge Checklist