Skip to content

fix: prevent reserved variable names in tests#785

Merged
bshaffer merged 1 commit intomainfrom
fix-variablename-collision-in-tests
Oct 27, 2025
Merged

fix: prevent reserved variable names in tests#785
bshaffer merged 1 commit intomainfrom
fix-variablename-collision-in-tests

Conversation

@bshaffer
Copy link
Copy Markdown
Collaborator

See googleapis/google-cloud-php#8683

Tests are failing with the following error:
Screenshot 2025-10-23 at 3 29 10 PM

This is because in one of the generated tests, the $status local variable in the test is being overwritten with a variable named $status that is used to populate a required field in the request:

    public function searchAdReviewCenterAdsExceptionTest()
    {
        //...
        $status = new stdClass();
        $status->code = Code::DATA_LOSS;
        $status->details = 'internal error';
        // ...
        $status = AdReviewCenterAdStatus::AD_REVIEW_CENTER_AD_STATUS_UNSPECIFIED;
        $request = (new SearchAdReviewCenterAdsRequest())->setParent($formattedParent)->setStatus($status);
        try {
            $gapicClient->searchAdReviewCenterAds($request);
            // If the $gapicClient method call did not throw, fail the test
            $this->fail('Expected an ApiException, but no exception was thrown.');
        } catch (ApiException $ex) {
            $this->assertEquals($status->code, $ex->getCode());
            $this->assertEquals($expectedExceptionMessage, $ex->getMessage());
        }
        //...
    }

I've fixed this by passing an array of $reservedNames to the code which generates this, and prefixing those names with request_ before they're converted to camelcased. This will result in the name being $requestStatus in this case.

@bshaffer bshaffer force-pushed the fix-variablename-collision-in-tests branch from 9ad9168 to f166467 Compare October 23, 2025 22:40
// Mock request
$aString = 'aString-929604177';
$partOfRequestA = [];
$requestStatus = 892481550;
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it worked!

@bshaffer bshaffer marked this pull request as ready for review October 23, 2025 22:48
@bshaffer bshaffer requested review from a team October 23, 2025 22:48
@bshaffer bshaffer merged commit ac546b1 into main Oct 27, 2025
9 checks passed
@bshaffer bshaffer deleted the fix-variablename-collision-in-tests branch October 27, 2025 15:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants