Open
Conversation
Contributor
|
Oxygen deployed a preview of your
Learn more about Hydrogen's GitHub integration. |
|
🤖 Code Review · #projects-dev-ai for questions ✅ Complete - No issues 📋 History✅ No issues |
kdaviduik
approved these changes
Mar 6, 2026
… baseURL Address PR review feedback from kdaviduik: - Make `capturedUrl` private so `getUrl()` is the sole public access point - Make `start()` private since it's only called from the constructor, preventing misuse if called post-stop() - Remove redundant `this.capturedUrl = startupPromise` inside start() - the constructor assignment is the single write point. The `.catch()` now references `startupPromise` directly because `this.capturedUrl` hasn't been assigned yet during start() execution (the constructor assignment happens after start() returns) - Add explicit guard in index.ts throwing if baseURL is falsy, surfacing a clear error instead of silently passing undefined to Playwright
…stability # Conflicts: # e2e/fixtures/server.ts
On main, beforeAll ended with `await server.start()` which blocked until the server was ready. After making start() private (called eagerly in constructor), beforeAll returned immediately without waiting. This caused the baseURL fixture to see server as null in parallel worker configurations. Restore the barrier with `await server.getUrl()` at the end of beforeAll. Also remove the overly aggressive undefined guard that was masking the real error in CI.
itsjustriley
approved these changes
Mar 20, 2026
…stability # Conflicts: # e2e/fixtures/index.ts # e2e/fixtures/server.ts
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
3 participants
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.
WHY are these changes introduced?
Fixes intermittent issues locally when the server URL is requested before it had time to start, and if it didn’t have a hardcoded port it would throw an error
WHAT is this pull request doing?
HOW to test your changes?
Checklist