Skip to content

Backfill test coverage from 66% to 93%#245

Open
AndyWendt wants to merge 3 commits intomasterfrom
backfill-test-coverage
Open

Backfill test coverage from 66% to 93%#245
AndyWendt wants to merge 3 commits intomasterfrom
backfill-test-coverage

Conversation

@AndyWendt
Copy link
Member

@AndyWendt AndyWendt commented Mar 21, 2026

Summary

  • Adds 67 new tests across 6 new test files and 3 modified test files, raising line coverage from 66% to 93%
  • Achieves 100% line coverage on 8 of 10 source classes (Config, ConfigTrait, ConfigRetriever, OAuth1\Server, OAuth1\User, OAuth2\AbstractProvider, OAuth2\User, Exceptions)
  • Adds HTTP client injection seam to OAuth1ServerStub via createHttpClient() override, matching the existing OAuth2 test pattern

Coverage by class

Class Before After
OAuth1\Server 0% 100%
OAuth1\User 0% 100%
Config 90% 100%
ConfigTrait 70% 100%
ConfigRetriever 82% 100%
OAuth2\AbstractProvider 88% 100%
OAuth1\AbstractProvider 4% 83%
ServiceProvider 25% 58%

Uncovered lines (14 total)

Dead code — stateful session paths (8 lines): OAuth1\AbstractProvider lines 94-97 and 163-168 use getSession()->put()/get(), but SymfonySessionDecorator in modern Laravel does not expose put(). The $stateless property defaults to true, making these paths unreachable.

Requires Lumen (5 lines): ServiceProvider:38-39 (class_exists('Laravel\Lumen\Application')) and SocialiteWasCalled:61 (SOCIALITEPROVIDERS_STATELESS constant).

Constant cannot be toggled (1 line): OAuth1\AbstractProvider:186-187 — once SOCIALITEPROVIDERS_STATELESS is defined, PHP constants cannot be undefined within a process.

Test plan

  • All 95 tests pass on PHP 8.4
  • No changes to production source code
  • Existing 28 tests unaffected
  • CI passes across PHP 8.2–8.5

🤖 Generated with Claude Code

AndyWendt and others added 3 commits March 21, 2026 12:24
Add comprehensive tests for previously untested or under-tested classes:

- OAuth1\Server: 0% → 100% (getTokenCredentials happy path, MITM check,
  bad response handling, legacy Guzzle branch, scopes, with, formatScopes)
- OAuth1\AbstractProvider: 4% → 83% (user flow, redirect, stateless,
  scopes/with delegation, setConfig, serviceContainerKey, error paths)
- OAuth1\User: 0% → 100%
- Config: 90% → 100% (relative URI resolution via URL::to facade)
- ConfigTrait: 70% → 100% (getConfig edge cases for falsy values)
- ConfigRetriever: 82% → 100% (console spoofing branch, missing guzzle
  default, missing required keys, additional config keys)
- OAuth2\AbstractProvider: 88% → 100% (parseApprovedScopes for array,
  string, null, numeric, and boolean inputs)
- ServiceProvider: added Lumen boot path and register() tests

Remaining uncovered lines are dead code (stateful session paths broken
with modern Laravel's SymfonySessionDecorator) or require Lumen to be
installed.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Split the test step so PHP 8.5 runs with --coverage-text (prints a
coverage summary table in the CI log) and --coverage-clover (generates
the Clover XML file for Codecov). Non-coverage runs skip the coverage
driver overhead entirely.

Also explicitly point the Codecov action to the Clover output file.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Switch from xdebug to pcov (faster, lower overhead) and run coverage
on all PHP versions instead of only 8.5. Every CI run now prints a
coverage summary table in the log. Codecov upload remains on 8.5 only
to avoid duplicate reports.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copy link
Member

@atymic atymic left a comment

Choose a reason for hiding this comment

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

Could you have a quick check of the generated tests are remove the ones testing the internal behaviour?

use SocialiteProviders\Manager\OAuth1\User;
use SocialiteProviders\Manager\Test\Stubs\OAuth1ServerStub;

class TestableOAuth1Provider extends OAuth1AbstractProvider
Copy link
Member

Choose a reason for hiding this comment

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

Could we split this out into its own class? Don't like the double classes in one file

/**
* @test
*/
public function stateless_defaults_to_true_argument(): void
Copy link
Member

Choose a reason for hiding this comment

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

do we really need to test stuff like this?

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