Fix failing intergration test (RAD-1066) #72
Conversation
foglcz
requested review from
kubasimon,
davidkmenta,
OndraM,
jirinovak,
legendik,
kdosiodjinud,
VincTheSecond,
MortalFlesh,
hokypierce,
michalrychlik,
MarketaSebkova,
florianjiri and
a user
|
|
||
| /** | ||
| * @covers \Lmc\Matej\RequestBuilder\EventsRequestBuilder | ||
| */ | ||
| class EventsRequestBuilderTest extends IntegrationTestCase | ||
| { | ||
| protected function setup(): void |
There was a problem hiding this comment.
This should be setUp, and I'm wondering why this was not detected by codestyle check, there should be one exactly for this 🤔
| { | ||
| $builder = $this->createMatejInstance()->request()->setupItemProperties(); | ||
|
|
||
| $this->buildAndSendPropertySetupRequest($builder); |
There was a problem hiding this comment.
Is it OK this will be done before each test method in this testcase?
|
|
||
| private function buildAndSendPropertySetupRequest(ItemPropertiesSetupRequestBuilder $builder): void | ||
| { | ||
| $builder->addProperty(ItemPropertySetup::string('test_property_a')) |
There was a problem hiding this comment.
We may add the identifier to some private class constants, to signalize these are "predefined" item properties available for use in this test. What do you thing?
|
|
||
| /** | ||
| * @covers \Lmc\Matej\RequestBuilder\EventsRequestBuilder | ||
| */ | ||
| class EventsRequestBuilderTest extends IntegrationTestCase | ||
| { | ||
| protected function setup(): void |
| @@ -32,21 +32,21 @@ protected function createMatejInstance(): Matej | |||
| return $instance; | |||
| } | |||
|
|
|||
| protected function assertResponseCommandStatuses(Response $response, ...$expectedCommandStatuses): void | |||
| protected static function assertResponseCommandStatuses(Response $response, ...$expectedCommandStatuses): void | |||
There was a problem hiding this comment.
Why is this static? This should be non-static, co it could be used in a same way as other phpunit asserts (via $this->assertResponseCommandStatuses, not static::assertResponseCommandStatuses)
There was a problem hiding this comment.
This is what happens then:
So if we don't need to, lets keep this non-static :)
| } | ||
| } | ||
|
|
||
| protected function createMatejInstance(): Matej | ||
| protected static function createMatejInstance(): Matej |
There was a problem hiding this comment.
If you declare the method static, you should use it via static call eveywhere where it is being used (in all other testcases)... Like static::createMatejInstance()
| @@ -32,21 +32,21 @@ protected function createMatejInstance(): Matej | |||
| return $instance; | |||
| } | |||
|
|
|||
| protected function assertResponseCommandStatuses(Response $response, ...$expectedCommandStatuses): void | |||
| protected static function assertResponseCommandStatuses(Response $response, ...$expectedCommandStatuses): void | |||
There was a problem hiding this comment.
This is what happens then:
So if we don't need to, lets keep this non-static :)
foglcz
force-pushed
the
bugfix/integration-tests
branch
from
cf93778 to
020b94d
Compare
|
Nice job @foglcz, thanks! 👍 |
Fixes failing integration test, which was failing because of uninitialized item properties.