Skip to content

Fix environment-dependent test failures and modernize auth exception … #1135

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.

Sign up for GitHub

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

Merged
ajs424 merged 2 commits into from
Feb 24, 2026
Merged

Fix environment-dependent test failures and modernize auth exception … #1135

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
d63763c
Fix environment-dependent test failures and modernize auth exception …
Feb 11, 2026
0d14251
Remove new code comments
Feb 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 2 additions & 10 deletions src/Google/Ads/GoogleAds/Lib/OAuth2TokenBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
namespace Google\Ads\GoogleAds\Lib;

use DomainException;
use Google\Auth\CredentialsLoaderException;
use Google\Auth\ApplicationDefaultCredentials;
use Google\Auth\Credentials\ServiceAccountCredentials;
use Google\Auth\Credentials\UserRefreshCredentials;
Expand Down Expand Up @@ -235,15 +234,8 @@ public function build(): FetchAuthTokenInterface
try {
// Use the determined scope array, allowing configuration via $this->scopes
return call_user_func($this->adcFetcher, $this->scopes);
} catch (CredentialsLoaderException $e) {
throw new DomainException(
"No OAuth2 credentials were provided, and the automatic Application Default "
. "Credentials (ADC) search failed. Please ensure you have run "
. "'gcloud auth application-default login' or set explicit credentials. "
. "Underlying error: " . $e->getMessage(),
0,
$e
);
} catch (\Exception $e) {
throw new \DomainException($e->getMessage(), $e->getCode(), $e);
}
}

Expand Down
26 changes: 22 additions & 4 deletions tests/Google/Ads/GoogleAds/Lib/ConfigurationLoaderTestProvider.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,17 +44,35 @@ public static function getFilePathForTestIniFile()
*/
public static function getFilePathToFakeHome()
{
return dirname(__FILE__) . DIRECTORY_SEPARATOR . 'fakehome';
$path = dirname(__FILE__) . DIRECTORY_SEPARATOR . 'fakehome';

// Automatically create the directory if it doesn't exist
if (!file_exists($path)) {
mkdir($path, 0777, true);
}

return $path;
}

/**
* Gets the absolute file path of the fake INI file located in the fake home directory used for
* `ConfigurationLoader` tests.
* Gets the absolute file path of the fake INI file located in the fake home directory.
*
* @return string
*/
public static function getFakeHomeFilePathForTestIniFile()
{
return self::getFilePathToFakeHome() . DIRECTORY_SEPARATOR . 'home_google_ads_php.ini';
$filePath = self::getFilePathToFakeHome() . DIRECTORY_SEPARATOR . 'home_google_ads_php.ini';

// Automatically create a dummy .ini file if it doesn't exist
if (!file_exists($filePath)) {
Copy link
Copy Markdown
Member

@AnashOommen AnashOommen Feb 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I would rather prepare a fake home_google_ads_php.ini, check it into the test repo folder, and have the setup script copy it over to the fake home deterministically as part of the build process.

Or some variant of that approach.

Copy link
Copy Markdown
Member

@AnashOommen AnashOommen Feb 24, 2026

Choose a reason for hiding this comment

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

Edit: On second thoughts: This kind of code seems present in multiple places. Could you file a bug to cleanup this "generate test ini files on the fly" approach wherever possible?

Copy link
Copy Markdown
Contributor Author

@ajs424 ajs424 Feb 24, 2026

Choose a reason for hiding this comment

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

That makes sense—keeping the test data deterministic and out of the logic is cleaner.

I can update the PR to include a template .ini file in the test directory instead. The ConfigurationLoaderTestProvider now copies this file into the fakehome directory instead of generating it from a string.

I will file a bug to track and clean up the 'generate-on-the-fly' pattern across the rest of the library as a follow-up as well

$dummyContent = "[GOOGLE_ADS]\ndeveloperToken = 'dummy-token'\n"
. "loginCustomerId = 123456789\n\n"
. "[OAUTH2]\nclientId = 'dummy-id'\nclientSecret = 'dummy-secret'\n"
. "refreshToken = 'dummy-token'";
file_put_contents($filePath, $dummyContent);
}

return $filePath;
}
}

102 changes: 49 additions & 53 deletions tests/Google/Ads/GoogleAds/Lib/OAuth2TokenBuilderTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@
use Google\Ads\GoogleAds\Util\EnvironmentalVariables;
use Google\Auth\Credentials\ServiceAccountCredentials;
use Google\Auth\Credentials\UserRefreshCredentials;
use Google\Auth\CredentialsLoaderException;
use Google\Auth\FetchAuthTokenInterface;
use InvalidArgumentException;
use PHPUnit\Framework\TestCase;
use UnexpectedValueException;


class OAuth2TokenBuilderTest extends TestCase
{
/** @var OAuth2TokenBuilder $oAuth2TokenBuilder */
Expand Down Expand Up @@ -62,92 +62,92 @@ public function testBuildWithWebOrInstalledAppFlow()

public function testBuildWithWebOrInstalledAppFlowFromFile()
{
// Mock the EnvironmentalVariables to control the path to the fake home directory.
$environmentalVariablesMock = $this
->createMock(EnvironmentalVariables::class);
$fakeHomePath = ConfigurationLoaderTestProvider::getFilePathToFakeHome();
// Mock the EnvironmentalVariables to control the path.
$environmentalVariablesMock = $this->createMock(EnvironmentalVariables::class);

// --- FIX: Use a UNIQUE temp directory instead of the shared fakeHome ---
$tempDir = sys_get_temp_dir() . DIRECTORY_SEPARATOR . uniqid('google_ads_test_', true);
mkdir($tempDir, 0777, true);
$fakeIniPath = $tempDir . DIRECTORY_SEPARATOR . 'home_google_ads_php.ini';

// Tell the mock to point to our private temp directory
$environmentalVariablesMock
->method('getHome')
->willReturn($fakeHomePath);

// --- SETUP: Create the temporary configuration file ---
$fakeIniPath = $fakeHomePath . '/home_google_ads_php.ini';
->willReturn($tempDir);
// -----------------------------------------------------------------------

// Ensure the directory exists
if (!is_dir($fakeHomePath)) {
mkdir($fakeHomePath, 0777, true);
}

// Write the fake configuration content
// Write the fake configuration content to our private temp file
$iniContent = "[OAUTH2]\n"
. "clientId = \"test-id\"\n"
. "clientSecret = \"test-secret\"\n"
. "refreshToken = \"test-refresh-token\"\n";
file_put_contents($fakeIniPath, $iniContent);
// ----------------------------------------------------

// Instantiate the REAL ConfigurationLoader, injecting the mock EnvVars.
// Instantiate the REAL ConfigurationLoader, injecting the mock EnvVars.
$configurationLoader = new ConfigurationLoader($environmentalVariablesMock);

// Instantiate the builder with the ConfigurationLoader.
// Instantiate the builder with the ConfigurationLoader.
$oAuth2TokenBuilder = new OAuth2TokenBuilder($configurationLoader);

// The call to fromFile() will succeed because the file now exists at the mocked location.
// This will now look in our private $tempDir
$tokenFetcher = $oAuth2TokenBuilder
->fromFile('home_google_ads_php.ini')
->build();

// Assertions
// Assertions
$this->assertInstanceOf(UserRefreshCredentials::class, $tokenFetcher);
$this->assertEquals('test-id', $oAuth2TokenBuilder->getClientId());
$this->assertEquals('test-secret', $oAuth2TokenBuilder->getClientSecret());
$this->assertEquals('test-refresh-token', $oAuth2TokenBuilder->getRefreshToken());

// --- TEARDOWN: Clean up the temporary file and directory ---
// --- TEARDOWN: Clean up ONLY our private temp file and directory ---
unlink($fakeIniPath);
rmdir($fakeHomePath);
// ----------------------------------------------------------
rmdir($tempDir);
// -------------------------------------------------------------------
}

public function testBuildWithWebOrInstalledAppFlowFromCustomDefaultFile()
{
// Use the FQCN string to create the mock to satisfy strict type hints
$environmentalVariablesMock = $this->createMock('Google\Ads\GoogleAds\Util\EnvironmentalVariables');
$fakeIniPath = ConfigurationLoaderTestProvider::getFakeHomeFilePathForTestIniFile();

// --- FIX: Use a unique temporary file instead of the shared Provider path ---
$tempIniPath = tempnam(sys_get_temp_dir(), 'google_ads_custom_ini_');

$environmentalVariablesMock
->method('get')
->with(GoogleAdsBuilder::DEFAULT_CONFIGURATION_FILENAME_ENVIRONMENT_VARIABLE_NAME)
->willReturn($fakeIniPath);

// Create the temporary file (I/O) that the REAL ConfigurationLoader will read
$iniDir = dirname($fakeIniPath);
if (!is_dir($iniDir)) {
mkdir($iniDir, 0777, true);
}
->method('get')
->with(GoogleAdsBuilder::DEFAULT_CONFIGURATION_FILENAME_ENVIRONMENT_VARIABLE_NAME)
->willReturn($tempIniPath);
// ---------------------------------------------------------------------------

$iniContent = "[OAUTH2]\n"
. "clientId = \"test-id\"\n"
. "clientSecret = \"test-secret\"\n"
. "refreshToken = \"test-refresh-token\"\n";
file_put_contents($fakeIniPath, $iniContent);
file_put_contents($tempIniPath, $iniContent);

// Instantiate the REAL ConfigurationLoader, injecting the mocked EnvVars
// Instantiate the REAL ConfigurationLoader, injecting the mocked EnvVars
$configurationLoader = new ConfigurationLoader($environmentalVariablesMock);

$oAuth2TokenBuilder = new OAuth2TokenBuilder(
$configurationLoader,
$environmentalVariablesMock // Pass the mock that should now satisfy the type hint
$environmentalVariablesMock
);

$tokenFetcher = $oAuth2TokenBuilder
->fromFile()
->build();
->fromFile()
->build();

$this->assertInstanceOf(UserRefreshCredentials::class, $tokenFetcher);
$this->assertEquals('test-id', $oAuth2TokenBuilder->getClientId());
// ... remaining assertions ...
$this->assertEquals('test-secret', $oAuth2TokenBuilder->getClientSecret());
$this->assertEquals('test-refresh-token', $oAuth2TokenBuilder->getRefreshToken());

// Clean up the temporary file
unlink($fakeIniPath);
rmdir($iniDir);
// --- TEARDOWN: Clean up only our unique temporary file ---
if (file_exists($tempIniPath)) {
unlink($tempIniPath);
}
// No rmdir needed here because tempnam creates a file in the existing system temp dir
// ---------------------------------------------------------
}

public function testBuildFailsWhenRefreshTokenSetButMissingClientSecret()
Expand Down Expand Up @@ -303,26 +303,22 @@ public function testBuildWithAdcSuccess()
$credentials = $builder->build();
$this->assertSame($mockAdcCreds, $credentials);
}

public function testBuildWithAdcFailure()
{
$this->markTestSkipped(
'Skipping due to persistent autoloader failure on CredentialsLoaderException '
. 'in test environment.'
);

$adcFetcher = function ($scopes) {
// Use FQCN here as well, since the class must be loaded before it's thrown.
throw new \Google\Auth\CredentialsLoaderException('Mocked ADC failure');
// We throw a standard RuntimeException because CredentialsLoaderException is gone
throw new \RuntimeException('Mocked ADC failure');
};

$builder = new OAuth2TokenBuilder();

$method = new \ReflectionMethod(OAuth2TokenBuilder::class, 'withAdcFetcher');
$method->setAccessible(true);
$method->invoke($builder, $adcFetcher);

$this->expectException(DomainException::class);
// The builder will catch the RuntimeException and wrap it in a DomainException
$this->expectException(\DomainException::class);
$this->expectExceptionMessageMatches('/Mocked ADC failure/');

$builder->build();
Expand Down