-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix(cypress): Chainlit shutdown and entrypoint checks #2728
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 2 files
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Ask questions if you need clarification on any suggestion
|
@coterp Thank you for contribution! @hayescode @sandangel I would like to review this PR myself, as I tried to implement such functionality earlier and had an issues with that |
|
@hayescode Sounds good, thanks for your review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@coterp Unfortunately I have same issue with it as when I tried implement such feature myself: it doesn't work in Cypress interactive mode, at least on Linux
Try to:
- Open Cypress in interactive mode (for example, via
pnpm test:interactive) - Run
custom_themetests (you'll see green screen, then red) - Run
data_layertests (you'll see green screen again and tests will fail because login isn't enabled in previous test server, while it is indata_layertest server)
|
But I think we can merge some implements from here, like broken entrypoint file existence check, if you'll remove or fix new server restart code |
Summary
This PR improves the reliability of Cypress E2E runs when starting and stopping Chainlit.
Changes
fkill) only as a fallback when no tracked process existsfs/promises.accesswas not awaited)cypress.config.tsnode:+typeimports)Motivation
Cypress E2E runs were failing consistently due to unreliable Chainlit shutdown and restart behavior in containerized dev environments.
Tests frequently failed with port-in-use errors because previous Chainlit instances were not reliably terminated between runs or between specs. The existing port-based termination (
fkill(:port)) proved unreliable in containers and was insufficient as a primary means of terminating Chainlit.This PR introduces explicit process tracking and shutdown of the spawned Chainlit process, using port-based termination only as a fallback to clean up stray instances from previous runs.
Testing
uv run pytest --cov=chainlitpnpm test(Cypress, headless via Xvfb)pnpm test:uipnpm lintSummary by cubic
Improves reliability of Cypress E2E runs by cleanly starting and stopping Chainlit to avoid port-in-use errors in containers. Tracks and kills the spawned process group, using port-based termination only as a fallback.
Written for commit d8e9a68. Summary will update automatically on new commits.