Skip to content

fix(php): use WIREMOCK_URL env var in Wire test setUp method#13627

Merged
iamnamananand996 merged 1 commit intomainfrom
devin/1773756643-php-wire-test-wiremock-url
Mar 17, 2026
Merged

fix(php): use WIREMOCK_URL env var in Wire test setUp method#13627
iamnamananand996 merged 1 commit intomainfrom
devin/1773756643-php-wire-test-wiremock-url

Conversation

@iamnamananand996
Copy link
Contributor

Description

Fixes PHP SDK Wire test files hardcoding http://localhost:8080 for the WireMock URL instead of reading from the WIREMOCK_URL environment variable. This prevented tests from working with dynamically assigned Docker ports in local development and CI/CD environments.

Changes Made

  • Added $wiremockUrl = getenv('WIREMOCK_URL') ?: 'http://localhost:8080' variable declaration at the top of the generated setUp() method
  • Multi-URL environment path (the actual bug): Changed Environments::custom('http://localhost:8080', 'http://localhost:8080') to Environments::custom($wiremockUrl, $wiremockUrl)
  • Single-URL environment path (refactor): Changed inline getenv('WIREMOCK_URL') ?: 'http://localhost:8080' to use the shared $wiremockUrl variable
  • Added version 2.2.1 entry in generators/php/sdk/versions.yml
  • Updated all 15 seed snapshot files in seed/php-sdk/exhaustive/wire-tests/

Review Checklist

  • Verify getMultiUrlEnvironmentForTest() — its return values are now effectively ignored (only the count of URLs matters for determining the number of $wiremockUrl args in Environments::custom()). This is intentional but worth confirming.
  • No seed fixture currently exercises the multi-URL environment code path, which is the path that was actually broken. The single-URL path already worked. The fix is straightforward but untested in seeds.

Testing

  • Seed tests regenerated and passing (php-sdk / exhaustive / wire-tests)
  • Lint (pnpm run check) passing

Link to Devin session: https://app.devin.ai/sessions/7681b11c333f4ede905a09ca0a8e40cc
Requested by: @iamnamananand996

Wire test files now read the WIREMOCK_URL environment variable with a
fallback to http://localhost:8080 instead of hardcoding the URL. This
enables tests to work with dynamically assigned Docker ports in local
development and CI/CD environments.

The setUp() method now declares a $wiremockUrl variable at the top that
is used for both single-URL (via options baseUrl) and multi-URL
(via Environments::custom()) environment configurations.
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@claude
Copy link

claude bot commented Mar 17, 2026

⚠️ Code review skipped — your organization's overage spend limit has been reached.

Code review is billed via overage credits. To resume reviews, an organization admin can raise the monthly limit in Settings → Usage.

Once credits are available, reopen this pull request to trigger a review.

@iamnamananand996 iamnamananand996 changed the title fix(php): use WIREMOCK_URL env var in Wire test setUp method fix(php): use WIREMOCK_URL env var in Wire test setUp method Mar 17, 2026
@iamnamananand996 iamnamananand996 merged commit 739cfe6 into main Mar 17, 2026
87 of 88 checks passed
@iamnamananand996 iamnamananand996 deleted the devin/1773756643-php-wire-test-wiremock-url branch March 17, 2026 17:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants