Skip to content
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

ref: Make getScope private #776

Merged
merged 5 commits into from
Feb 22, 2019
Merged

ref: Make getScope private #776

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5879d5c
ref: Make getScope private
HazAT Feb 22, 2019
6ea8995
ref: Codereview
HazAT Feb 22, 2019
0534e85
Update changelog [skip CI]
Jean85 Feb 22, 2019
838d529
ref: CR
HazAT Feb 22, 2019
6d142d6
Simplify reflection calls
Jean85 Feb 22, 2019
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
18 changes: 10 additions & 8 deletions src/State/Hub.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,14 +52,6 @@ public function getClient(): ?ClientInterface
return $this->getStackTop()->getClient();
}

/**
* {@inheritdoc}
*/
public function getScope(): Scope
{
return $this->getStackTop()->getScope();
}

/**
* {@inheritdoc}
*/
Expand Down Expand Up @@ -242,6 +234,16 @@ public function getIntegration(string $className): ?IntegrationInterface
return null;
}

/**
* Gets the scope bound to the top of the stack.
*
* @return Scope
*/
private function getScope(): Scope
{
return $this->getStackTop()->getScope();
}

/**
* Gets the topmost client/layer pair in the stack.
*
Expand Down
7 changes: 0 additions & 7 deletions src/State/HubInterface.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,6 @@ interface HubInterface
*/
public function getClient(): ?ClientInterface;

/**
* Gets the scope bound to the top of the stack.
*
* @return Scope
*/
public function getScope(): Scope;

/**
* Gets the ID of the last captured event.
*
Expand Down
10 changes: 8 additions & 2 deletions tests/ClientTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -274,8 +274,14 @@ public function testHandlingExceptionThrowingAnException(): void

$client = new Client(new Options(), $transport, $this->createEventFactory());

Hub::getCurrent()->bindClient($client);
$client->captureException($this->createCarelessExceptionWithStacktrace(), Hub::getCurrent()->getScope());
$hub = Hub::getCurrent();
$hub->bindClient($client);

$class = new \ReflectionClass($hub);
HazAT marked this conversation as resolved.
Show resolved Hide resolved
$method = $class->getMethod('getScope');
$method->setAccessible(true);

$client->captureException($this->createCarelessExceptionWithStacktrace(), $method->invokeArgs($hub, []));
}

/**
Expand Down
8 changes: 7 additions & 1 deletion tests/SdkTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,13 @@ public function testAddBreadcrumb(): void

addBreadcrumb($breadcrumb);

$this->assertSame([$breadcrumb], Hub::getCurrent()->getScope()->getBreadcrumbs());
$hub = Hub::getCurrent();

$class = new \ReflectionClass($hub);
HazAT marked this conversation as resolved.
Show resolved Hide resolved
$method = $class->getMethod('getScope');
$method->setAccessible(true);

$this->assertSame([$breadcrumb], $method->invokeArgs($hub, [])->getBreadcrumbs());
}

public function testWithScope(): void
Expand Down
41 changes: 25 additions & 16 deletions tests/State/HubTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ public function testConstructorCreatesScopeAutomatically(): void
{
$hub = new Hub(null, null);

$this->assertNotNull($hub->getScope());
$this->assertNotNull(self::getScope($hub));
}

public function testGetClient(): void
Expand All @@ -36,7 +36,7 @@ public function testGetScope(): void
$scope = new Scope();
$hub = new Hub($this->createMock(ClientInterface::class), $scope);

$this->assertSame($scope, $hub->getScope());
$this->assertSame($scope, self::getScope($hub));
}

public function testGetLastEventId(): void
Expand All @@ -57,14 +57,14 @@ public function testPushScope(): void
{
$hub = new Hub($this->createMock(ClientInterface::class));

$scope1 = $hub->getScope();
$scope1 = self::getScope($hub);
$client1 = $hub->getClient();

$scope2 = $hub->pushScope();
$client2 = $hub->getClient();

$this->assertNotSame($scope1, $scope2);
$this->assertSame($scope2, $hub->getScope());
$this->assertSame($scope2, self::getScope($hub));
$this->assertSame($client1, $client2);
$this->assertSame($client1, $hub->getClient());
}
Expand All @@ -73,22 +73,22 @@ public function testPopScope(): void
{
$hub = new Hub($this->createMock(ClientInterface::class));

$scope1 = $hub->getScope();
$scope1 = self::getScope($hub);
$client = $hub->getClient();

$scope2 = $hub->pushScope();

$this->assertSame($scope2, $hub->getScope());
$this->assertSame($scope2, self::getScope($hub));
$this->assertSame($client, $hub->getClient());

$this->assertTrue($hub->popScope());

$this->assertSame($scope1, $hub->getScope());
$this->assertSame($scope1, self::getScope($hub));
$this->assertSame($client, $hub->getClient());

$this->assertFalse($hub->popScope());

$this->assertSame($scope1, $hub->getScope());
$this->assertSame($scope1, self::getScope($hub));
$this->assertSame($client, $hub->getClient());
}

Expand All @@ -97,7 +97,7 @@ public function testWithScope(): void
$scope = new Scope();
$hub = new Hub($this->createMock(ClientInterface::class), $scope);

$this->assertSame($scope, $hub->getScope());
$this->assertSame($scope, self::getScope($hub));

$callbackInvoked = false;

Expand All @@ -116,7 +116,7 @@ public function testWithScope(): void
}

$this->assertTrue($callbackInvoked);
$this->assertSame($scope, $hub->getScope());
$this->assertSame($scope, self::getScope($hub));
}

public function testConfigureScope(): void
Expand All @@ -134,7 +134,7 @@ public function testConfigureScope(): void
});

$this->assertTrue($callbackInvoked);
$this->assertSame($scope, $hub->getScope());
$this->assertSame($scope, self::getScope($hub));
}

public function testBindClient(): void
Expand Down Expand Up @@ -217,7 +217,7 @@ public function testAddBreadcrumb(): void

$hub->addBreadcrumb($breadcrumb);

$this->assertSame([$breadcrumb], $hub->getScope()->getBreadcrumbs());
$this->assertSame([$breadcrumb], self::getScope($hub)->getBreadcrumbs());
}

public function testAddBreadcrumbDoesNothingIfMaxBreadcrumbsLimitIsZero(): void
Expand All @@ -227,14 +227,14 @@ public function testAddBreadcrumbDoesNothingIfMaxBreadcrumbsLimitIsZero(): void

$hub->addBreadcrumb(new Breadcrumb(Breadcrumb::LEVEL_ERROR, Breadcrumb::TYPE_ERROR, 'error_reporting'));

$this->assertEmpty($hub->getScope()->getBreadcrumbs());
$this->assertEmpty(self::getScope($hub)->getBreadcrumbs());
}

public function testAddBreadcrumbRespectsMaxBreadcrumbsLimit(): void
{
$client = ClientBuilder::create(['max_breadcrumbs' => 2])->getClient();
$hub = new Hub($client);
$scope = $hub->getScope();
$scope = self::getScope($hub);

$breadcrumb1 = new Breadcrumb(Breadcrumb::LEVEL_WARNING, Breadcrumb::TYPE_ERROR, 'error_reporting', 'foo');
$breadcrumb2 = new Breadcrumb(Breadcrumb::LEVEL_WARNING, Breadcrumb::TYPE_ERROR, 'error_reporting', 'bar');
Expand All @@ -260,7 +260,7 @@ public function testAddBreadcrumbDoesNothingWhenBeforeBreadcrumbCallbackReturnsN

$hub->addBreadcrumb(new Breadcrumb(Breadcrumb::LEVEL_ERROR, Breadcrumb::TYPE_ERROR, 'error_reporting'));

$this->assertEmpty($hub->getScope()->getBreadcrumbs());
$this->assertEmpty(self::getScope($hub)->getBreadcrumbs());
}

public function testAddBreadcrumbStoresBreadcrumbReturnedByBeforeBreadcrumbCallback(): void
Expand All @@ -276,7 +276,7 @@ public function testAddBreadcrumbStoresBreadcrumbReturnedByBeforeBreadcrumbCallb

$hub->addBreadcrumb($breadcrumb1);

$this->assertSame([$breadcrumb2], $hub->getScope()->getBreadcrumbs());
$this->assertSame([$breadcrumb2], self::getScope($hub)->getBreadcrumbs());
}

public function testCaptureEvent(): void
Expand All @@ -292,4 +292,13 @@ public function testCaptureEvent(): void

$this->assertEquals('2b867534eead412cbdb882fd5d441690', $hub->captureEvent(['message' => 'test']));
}

private static function getScope($hub): Scope
HazAT marked this conversation as resolved.
Show resolved Hide resolved
{
$class = new \ReflectionClass($hub);
HazAT marked this conversation as resolved.
Show resolved Hide resolved
$method = $class->getMethod('getScope');
$method->setAccessible(true);

return $method->invokeArgs($hub, []);
}
}