Skip to content

Replace static return types with self in SheetsClient for improved mock testing #42

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

Merged
merged 3 commits into from
Jun 18, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 18, 2025

Changes

This PR replaces all static return types with self in the SheetsClient class and related components to make mock testing of chained methods easier.

Files Modified

  • src/Contracts/Factory.php - Updated interface declarations from static to self (10 methods)
  • src/SheetsClient.php - Changed method return types from static to self (6 methods)
  • src/Concerns/SheetsDrive.php - Changed method return types from static to self (1 method)
  • src/Concerns/SheetsValues.php - Changed method return types from static to self (4 methods)
  • src/Facades/Sheets.php - Updated @method annotations from static static to static self (11 annotations)

Methods Updated

  • setService(), setAccessToken(), spreadsheet(), spreadsheetByTitle(), sheet(), sheetById(), setDriveService(), range(), majorDimension(), valueRenderOption(), dateTimeRenderOption()

Why This Change?

The change from static to self improves mock testing capabilities because:

  • self refers to the exact class where it's defined, making it more predictable for mocking
  • static uses late static binding which can be harder to mock in inheritance scenarios
  • Method chaining behavior remains identical, but mocking frameworks can now more easily predict return types

Testing

  • ✅ All existing tests pass (39 tests, 49 assertions)
  • ✅ Code linting passes
  • ✅ Method chaining functionality verified to work correctly
  • ✅ No breaking changes to public API

Example Usage (unchanged)

// Method chaining works exactly the same
$data = Sheets::setAccessToken($token)
    ->spreadsheet('spreadsheet-id')
    ->sheet('Sheet1')
    ->range('A1:B10')
    ->get();

The functionality remains identical for end users, but developers writing tests can now mock chained method calls more reliably.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copy link

⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done

Copy link

✅ I finished the code review, and didn't find any security or code quality issues.

@Copilot Copilot AI changed the title [WIP] Change all static return types to self in SheetsClient.php Replace static return types with self in SheetsClient for improved mock testing Jun 18, 2025
@Copilot Copilot AI requested a review from kawax June 18, 2025 03:16
Copilot finished work on behalf of kawax June 18, 2025 03:16
@kawax
Copy link
Member

kawax commented Jun 18, 2025

@copilot
Fixed all test code that had been written separately and could not be chained.
Also changed to the modern expects().

$this->google->shouldReceive('getCache')->once()->andReturn(m::self());
$this->google->shouldReceive('clear')->once();
$this->google->expects('getCache->clear')->once();

Copy link
Contributor Author

Copilot AI commented Jun 18, 2025

@copilot
Fixed all test code that had been written separately and could not be chained.
Also changed to the modern expects().
...

I've implemented the modernization you mentioned. Updated all test code to use the modern expects() syntax and fixed the chained method calls. The specific example you provided (getCache()->clear()) is now properly tested with expects('getCache->clear'). All tests pass and the code is more maintainable. Commit: 0111f08

Copilot finished work on behalf of kawax June 18, 2025 03:40
Copy link

qltysh bot commented Jun 18, 2025

Diff Coverage: The code coverage on the diff in this pull request is 100.0%.

Total Coverage: This PR will not change total coverage.

🛟 Help
  • Diff Coverage: Coverage for added or modified lines of code (excludes deleted files). Learn more.

  • Total Coverage: Coverage for the whole repository, calculated as the sum of all File Coverage. Learn more.

  • File Coverage: Covered Lines divided by Covered Lines plus Missed Lines. (Excludes non-executable lines including blank lines and comments.)

    • Indirect Changes: Changes to File Coverage for files that were not modified in this PR. Learn more.

@kawax kawax marked this pull request as ready for review June 18, 2025 03:41
@kawax kawax merged commit 674f3ba into main Jun 18, 2025
7 checks passed
@kawax kawax deleted the copilot/fix-518fc231-fdad-4534-93ac-75dc3fca48ad branch June 18, 2025 03:41
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