Skip to content

Conversation

@ggh2e3
Copy link

@ggh2e3 ggh2e3 commented Aug 11, 2025

Q A
Is bugfix?
New feature?
Breaks BC?

Hi, I added some tests just to get started. Hope it can help

@rustamwin rustamwin added the type:test Test label Aug 11, 2025
@samdark samdark requested review from Copilot and vjik August 12, 2025 02:35
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds initial test coverage for the ResponseGrabber and MockServiceProvider classes in the Yiisoft Yii Testing package. The tests help establish baseline functionality verification and error handling scenarios.

  • Adds comprehensive test coverage for ResponseGrabber class functionality
  • Adds basic test coverage for MockServiceProvider class methods
  • Introduces tests for both successful operations and error conditions

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
tests/ResponseGrabberTest.php Tests ResponseGrabber's response setting/getting functionality and null response error handling
tests/MockServiceProviderTest.php Tests MockServiceProvider's definition management and extensions retrieval
Comments suppressed due to low confidence (1)

tests/ResponseGrabberTest.php:17

  • [nitpick] The variable name 'responseStub' suggests a test double, but this is actually a real Response object. Consider renaming to 'response' or 'testResponse' for clarity.
        $responseStub = new Response(200, body: '{"key": "value"}', reason: 'Ok with body', version: '1.1');

@codecov
Copy link

codecov bot commented Aug 12, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 31.06%. Comparing base (9f56e05) to head (2652249).

Additional details and impacted files
@@             Coverage Diff              @@
##             master      #46      +/-   ##
============================================
+ Coverage     26.21%   31.06%   +4.85%     
  Complexity       60       60              
============================================
  Files             6        6              
  Lines           206      206              
============================================
+ Hits             54       64      +10     
+ Misses          152      142      -10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@samdark
Copy link
Member

samdark commented Aug 12, 2025

What exactly do these tests check?

@ggh2e3
Copy link
Author

ggh2e3 commented Aug 12, 2025

Hi @samdark,

They check the actual behavior of the tested classes. I checked the package list of Yii3 and saw that this one had little code coverage. I'd like to start contributing to open-source projects, so that’s why I chose it. I also tried to write more tests for the remaining classes, but it would require refactoring the production code to make them simple.

@samdark
Copy link
Member

samdark commented Aug 12, 2025

Production code?

@ggh2e3
Copy link
Author

ggh2e3 commented Aug 12, 2025

Production code?

Yeah, I mean, the application code

@samdark
Copy link
Member

samdark commented Aug 12, 2025

Which application code? I'm a bit lost. Could you elaborate?

@ggh2e3
Copy link
Author

ggh2e3 commented Aug 12, 2025

Sure,

by application code/production code, I mean the actual source code that makes the software work (the classes, functions, and logic under the src fodler) as opposed to test code (the code under the test folder).

I tried to write also other tests for the remaining classes but some of them are not unit-testable without refactoring them.

Example:
I drafted a test class to test src/FunctionalTestCase.php (an abstract class). I created an anonymous class inside the test that extends FunctionalTestCase and made the ?FunctionalTester $tester member public.

Then I started testing the methods. This could not be accomplished because FunctionalTester is a final class, and therefore it cannot be mocked. This makes the unit test impossible to write (or if possible, really complicated).

Two possible refactors to make the test easy to write are:

  1. Have FunctionalTester implement an interface that is used in the abstract class. In the test, the interface could be mocked instead of the concrete implementation.
  2. Remove final from FunctionalTester (not recommended, in my opinion).

Comment on lines +12 to +29
public function testGetDefinitionsWillReturnDefinitionsSetInArray(): void
{
$mockServiceProvider = new MockServiceProvider();
$mockServiceProvider->setDefinition('example-id-001', ['class' => 'Example1\Class1\Position1']);
$mockServiceProvider->setDefinition('example-id-002', 'Example1\Class2\Position2');
$mockServiceProvider->setDefinition('example-id-003', 'Example3\Class3\Position3');

$this->assertEquals([
'example-id-001' => ['class' => 'Example1\Class1\Position1'],
'example-id-002' => 'Example1\Class2\Position2',
'example-id-003' => 'Example3\Class3\Position3',
], $mockServiceProvider->getDefinitions());
}

public function testGetExtensionsWillReturnEmptyArray(): void
{
$this->assertEmpty((new MockServiceProvider())->getExtensions());
}
Copy link
Member

Choose a reason for hiding this comment

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

These tests do not correspond to real usage of the provider. Would you please make it closer to real usage?

Copy link
Author

Choose a reason for hiding this comment

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

Could you provide a link where I can find real usages? I'll check it and modify both tests

Comment on lines +15 to +41
public function testGetResponseWillReturnResponseAccessorSetResponse(): void
{
$response = new Response(200, body: '{"key": "value"}', reason: 'Ok with body', version: '1.1');

$responseGrabber = new ResponseGrabber();
$responseGrabber->setResponse($response);

$this->assertEquals(new ResponseAccessor($response), $responseGrabber->getResponse());
}

public function testGetResponseWillThrowExceptionIfSetResponseIsCalledWithNullParameter(): void
{
$this->expectException(RuntimeException::class);
$this->expectExceptionMessage('Response is null');

$responseGrabber = new ResponseGrabber();
$responseGrabber->setResponse(null);
$responseGrabber->getResponse();
}

public function testGetResponseWillThrowExceptionIfResponseIsNotSet(): void
{
$this->expectException(RuntimeException::class);
$this->expectExceptionMessage('Response is null');

(new ResponseGrabber())->getResponse();
}
Copy link
Member

Choose a reason for hiding this comment

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

These tests do not correspond to real usage. Would you please make it closer to real usage?

Copy link
Author

Choose a reason for hiding this comment

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

Could you provide a link where I can find real usages? I'll check it and modify both tests

@samdark
Copy link
Member

samdark commented Aug 13, 2025

I think there are none. @vjik could you provide some examples? Is it still there in app template or demos?

@vjik
Copy link
Member

vjik commented Aug 13, 2025

I think there are none. @vjik could you provide some examples? Is it still there in app template or demos?

I removed it from app template. @xepozz can you help with examples?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants