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

fix(test): prevent error by upgrading suites #512

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

fix(test): prevent error by upgrading suites #512

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
4965d42
fix(test): prevent error by upgrading suites
joerivanveen Nov 28, 2024
0370a28
fix: keep compatibility with php 7.1
joerivanveen Dec 5, 2024
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
3 changes: 2 additions & 1 deletion .github/workflows/--test.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ jobs:
if: github.actor != 'dependabot[bot]' && steps.cache-coverage.outputs.cache-hit != 'true'
id: docker
with:
image: ghcr.io/myparcelnl/php-xd:7.2
image: ghcr.io/myparcelnl/php-xd:8.2
Copy link
Contributor

Choose a reason for hiding this comment

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

Eigenlijk was de testsuite al niet geweldig want

A) We testen niet op alle PHP versies
B) We testen alleen op PHP 7.2 en niet op 7.1 welke volgens onze composer & README wel zou moeten werken

Hoe dan ook skippen we hier nu zoveel PHP versies dat het onmogelijk is om te weten of de boel in PHP < 8.2 nog werkt. Ik zou dit dan ook niet willen veranderen naar iets anders dan wat we in onze composer.json als versie meegeven (zie mijn comment daarbij)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Het klopt dat die niet geweldig is, wat ook niet geweldig is is dat die api verzoeken doet met random data, waardoor sommige tests soms falen maar niet altijd. Ik denk niet dat ik in een story van 0 punten dat allemaal kan fixen, daarom koos ik voor een upgrade zodat alles weer werkte en de andere SDK stories door kunnen.


- name: 'Install composer dependencies'
if: github.actor != 'dependabot[bot]' && steps.cache-coverage.outputs.cache-hit != 'true'
Expand All @@ -53,6 +53,7 @@ jobs:
--env CI=${CI} \
--env API_KEY_NL=${{ secrets.API_KEY_NL }} \
--env API_KEY_BE=${{ secrets.API_KEY_BE }} \
--env XDEBUG_MODE=coverage \
${{ steps.docker.outputs.image }} \
vendor/bin/phpunit --coverage-clover coverage.xml

Expand Down
8 changes: 4 additions & 4 deletions composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,14 @@
],
"scripts": {
"test": "phpunit",
"test:coverage": "vendor/bin/phpunit --coverage-html coverage"
"test:coverage": ["@putenv XDEBUG_MODE=coverage","vendor/bin/phpunit --coverage-html coverage"]
},
"require": {
"php": ">=7.1.0"
"php": ">=7.4.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Dit is een breaking change. In de README staat dat we momenteel PHP 7.1 ondersteunen. Voor klanten die < 7.4 hebben zou dan ineens de package niet meer installable of updateable zijn.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ja weet even niet waarom deze is aangepast, zal ik terugzetten.

},
"require-dev": {
"fakerphp/faker": "^v1.16.0",
"phpunit/phpunit": "^7.5.20",
"phpunit/phpunit": "^11",
"psr/container": "~1.0.0"
},
"autoload": {
Expand All @@ -50,4 +50,4 @@
"MyParcelNL\\Sdk\\Test\\": "test"
}
}
}
}
2 changes: 1 addition & 1 deletion docker-compose.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ version: '3.9'

# volumes are not in common, because phpstorm doesn't understand it
x-common: &common
image: ghcr.io/myparcelnl/php-xd:7.2
image: ghcr.io/myparcelnl/php-xd:8.2
Copy link
Contributor

Choose a reason for hiding this comment

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

Ditto als mijn opmerking in de test workflow, alleen dan toepasbaar op lokale ontwikkeling impact.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

zelfde antwoord

init: true
env_file:
- .env
Expand Down
8 changes: 4 additions & 4 deletions phpunit.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,11 @@
<directory>test</directory>
</testsuite>
</testsuites>
<filter>
<whitelist>
<source>
<include>
<directory suffix=".php">./src</directory>
</whitelist>
</filter>
</include>
</source>
<php>
<env name="TEST" value="true" />
</php>
Expand Down
4 changes: 4 additions & 0 deletions src/Model/Fulfilment/OrderLine.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,10 @@
$this->product = new Product($data['product'] ?? []);
}

public function __toString() {
return (string) self::class;

Check warning on line 77 in src/Model/Fulfilment/OrderLine.php

View check run for this annotation

Codecov / codecov/patch

src/Model/Fulfilment/OrderLine.php#L76-L77 

Added lines #L76 - L77 were not covered by tests
}

/**
* @return array
*/
Expand Down
6 changes: 5 additions & 1 deletion src/Support/Collection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1732,6 +1732,10 @@
return Helpers::toArrayWithoutNull($value);
}

if (is_int($value)) {
return $value;
}

if ($value && method_exists($value, 'toArrayWithoutNull')) {
return $value->toArrayWithoutNull();
}
Expand All @@ -1754,7 +1758,7 @@
public function jsonSerialize()
{
return array_map(function ($value) {
if (method_exists($value, 'toArray')) {
if (method_exists((string) $value, 'toArray')) {

Check warning on line 1761 in src/Support/Collection.php

View check run for this annotation

Codecov / codecov/patch

src/Support/Collection.php#L1761 

Added line #L1761 was not covered by tests
return $value->toArray();
}

Expand Down
2 changes: 1 addition & 1 deletion test/Bootstrap/ConsignmentTestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ protected function addCheckoutData(array $data, AbstractConsignment $consignment
*/
protected static function assertHasPdfLink(MyParcelCollection $collection): void
{
self::assertRegExp('#^https://api.myparcel.nl/pdfs#', $collection->getLinkOfLabels(), 'Can\'t get link of PDF');
self::assertMatchesRegularExpression('#^https://api.myparcel.nl/pdfs#', $collection->getLinkOfLabels(), 'Can\'t get link of PDF');
}

/**
Expand Down
11 changes: 7 additions & 4 deletions test/Bootstrap/TestCase.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,16 @@ class TestCase extends \PHPUnit\Framework\TestCase
protected $faker;

/**
* @param string $name
* @param array $data
* @param string $dataName
* @param string|null $name
* @param array $data
* @param string $dataName
*/
public function __construct($name = null, array $data = [], $dataName = '')
public function __construct(?string $name = null, array $data = [], $dataName = '')
{
$this->faker = Factory::create('nl_NL');
if (!$name) {
$name = self::class;
}
parent::__construct($name, $data, $dataName);
}

Expand Down
2 changes: 1 addition & 1 deletion test/Factory/Account/CarrierConfigurationFactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ class CarrierConfigurationFactoryTest extends TestCase
* @return array
* @throws \Exception
*/
public function provideCreateCarrierConfigurationData(): array
public static function provideCreateCarrierConfigurationData(): array
{
return [
// 'With identifier only' => [
Expand Down
9 changes: 5 additions & 4 deletions test/Factory/DeliveryOptionsAdapterFactoryTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ class DeliveryOptionsAdapterFactoryTest extends TestCase
* @return array
* @throws \Exception
*/
public function provideCreateData(): array
public static function provideCreateData(): array
{
$date = (new DateTime())->format('Ymd H:i:s');
return $this->createProviderDataset([
$instance = new self();
return $instance->createProviderDataset([
'DeliveryOptionsV2Adapter' => [
'carrier' => CarrierPostNL::NAME,
'date' => $date,
Expand Down Expand Up @@ -69,7 +70,7 @@ public function provideCreateData(): array
'age_check' => true,
'large_format' => true,
'return' => true,
'label_description' => $this->faker->words(3, true),
'label_description' => $instance->faker->words(3, true),
],
'pickupLocation' => null,
],
Expand All @@ -85,7 +86,7 @@ public function provideCreateData(): array
'age_check' => true,
'large_format' => true,
'return' => true,
'label_description' => $this->faker->words(3, true),
'label_description' => $instance->faker->words(3, true),
],
'pickupLocation' => [
'country' => AbstractConsignment::CC_NL,
Expand Down
6 changes: 3 additions & 3 deletions test/Helper/TrackTraceUrlTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,13 @@ class TrackTraceUrlTest extends TestCase
/**
* @return array
*/
public function provideTrackTraceData(): array
public static function provideTrackTraceData(): array
{
return [
'dutch destination' => [
'barcode' => '3SMYPA00123456',
'postal_code' => '2132JE',
'cc' => 'NL',
'postalCode' => '2132JE',
'countryCode' => 'NL',
],
];
}
Expand Down
2 changes: 1 addition & 1 deletion test/Helper/UtilsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class UtilsTest extends TestCase
/**
* @return array
*/
public function provideGetKeysWithoutValueData(): array
public static function provideGetKeysWithoutValueData(): array
{
return [
[
Expand Down
9 changes: 5 additions & 4 deletions test/Model/Consignment/BpostConsignmentTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,13 @@ class BpostConsignmentTest extends ConsignmentTestCase
* @return array
* @throws \Exception
*/
public function provideBpostConsignmentsData(): array
public static function provideBpostConsignmentsData(): array
{
return $this->createConsignmentProviderDataset([
$instance = new self();
return $instance->createConsignmentProviderDataset([
'BE -> BE' => [],
'BE -> NL' => $this->getDefaultAddress(),
'Bpost pickup + shipment options' => $this->getDefaultAddress(AbstractConsignment::CC_BE) + [
'BE -> NL' => $instance->getDefaultAddress(),
'Bpost pickup + shipment options' => $instance->getDefaultAddress(AbstractConsignment::CC_BE) + [
self::ONLY_RECIPIENT => true,
self::SIGNATURE => true,
self::DELIVERY_TYPE => AbstractConsignment::DELIVERY_TYPE_PICKUP,
Expand Down
18 changes: 10 additions & 8 deletions test/Model/Consignment/ConsignmentOtherOptionsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,11 @@ class ConsignmentOtherOptionsTest extends ConsignmentTestCase
* @return array
* @throws \Exception
*/
public function provideAutoDetectPickupData(): array
public static function provideAutoDetectPickupData(): array
{
$deliveryDate = $this->generateDeliveryDate();
return $this->createConsignmentProviderDataset(
$instance = new self();
$deliveryDate = $instance->generateDeliveryDate();
return $instance->createConsignmentProviderDataset(
[
'Auto detect pickup' => [
'checkout_data' => sprintf(
Expand All @@ -36,10 +37,11 @@ public function provideAutoDetectPickupData(): array
* @throws \Exception
* @deprecated
*/
public function provideCheckoutDataData(): array
public static function provideCheckoutDataData(): array
{
$deliveryDate = $this->generateDeliveryDate();
return $this->createConsignmentProviderDataset([
$instance = new self();
$deliveryDate = $instance->generateDeliveryDate();
return $instance->createConsignmentProviderDataset([
'[DEPRECATED] checkout data' => [
'checkout_data' => sprintf(
'{"date":"%s","time":[{"start":"16:00:00","type":4,"price":{"currency":"EUR","amount":0}}],"location":"Primera Sanders","street":"Polderplein","number":"3","postal_code":"2132BA","city":"Hoofddorp","cc":"NL","start_time":"16:00:00","price":0,"price_comment":"retail","comment":"Dit is een Postkantoor. Post en pakketten die u op werkdagen vóór de lichtingstijd afgeeft, bezorgen we binnen Nederland de volgende dag. Op zaterdag worden alléén pakketten die u afgeeft voor 15:00 uur maandag bezorgd.","phone_number":"","opening_hours":{"monday":["11:00-18:00"],"tuesday":["09:00-18:00"],"wednesday":["09:00-18:00"],"thursday":["09:00-18:00"],"friday":["09:00-21:00"],"saturday":["09:00-18:00"],"sunday":["12:00-17:00"]},"distance":"312","latitude":"52.30329367","longitude":"4.69476214","location_code":"176227","retail_network_id":"PNPNL-01","holiday":[]}',
Expand All @@ -61,9 +63,9 @@ public function provideCheckoutDataData(): array
* @return array
* @throws \Exception
*/
public function provideSaveRecipientAddressData(): array
public static function provideSaveRecipientAddressData(): array
{
return $this->createConsignmentProviderDataset([
return (new self())->createConsignmentProviderDataset([
'Save recipient address' => [
self::SAVE_RECIPIENT_ADDRESS => true,
],
Expand Down
53 changes: 29 additions & 24 deletions test/Model/Consignment/ConsignmentShipmentOptionsTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@ class ConsignmentShipmentOptionsTest extends ConsignmentTestCase
* @return array
* @throws \Exception
*/
public function provide18PlusCheckData(): array
public static function provide18PlusCheckData(): array
{
return $this->createConsignmentProviderDataset([
$instance = new self();
return $instance->createConsignmentProviderDataset([
'Normal 18+ check' => [
self::AGE_CHECK => true,
self::ONLY_RECIPIENT => true,
Expand All @@ -29,7 +30,7 @@ public function provide18PlusCheckData(): array
// self::expected(self::ONLY_RECIPIENT) => true,
// self::expected(self::SIGNATURE) => true,
// ],
'18+ check EU shipment' => $this->getDefaultAddress(AbstractConsignment::CC_BE) + [
'18+ check EU shipment' => $instance->getDefaultAddress(AbstractConsignment::CC_BE) + [
self::AGE_CHECK => true,
self::EXCEPTION => 'The age check is not possible with an EU shipment or world shipment',
],
Expand Down Expand Up @@ -68,9 +69,9 @@ public function provideDeliveryMomentData(): array
* @return array
* @throws \Exception
*/
public function provideDigitalStampData(): array
public static function provideDigitalStampData(): array
{
return $this->createConsignmentProviderDataset([
return (new self())->createConsignmentProviderDataset([
'Digital stamp 80 grams' => [
self::LABEL_DESCRIPTION => 112345,
self::PACKAGE_TYPE => AbstractConsignment::PACKAGE_TYPE_DIGITAL_STAMP,
Expand All @@ -95,11 +96,12 @@ public function provideDigitalStampData(): array
* @return array
* @throws \Exception
*/
public function provideLargeFormatData(): array
public static function provideLargeFormatData(): array
{
return $this->createConsignmentProviderDataset([
$instance = new self();
return $instance->createConsignmentProviderDataset([
'Large format national' => [
self::CUSTOMS_DECLARATION => $this->getDefaultCustomsDeclaration(),
self::CUSTOMS_DECLARATION => $instance->getDefaultCustomsDeclaration(),
self::LARGE_FORMAT => true,
],
// todo:
Expand All @@ -108,8 +110,8 @@ public function provideLargeFormatData(): array
// self::LARGE_FORMAT => true,
// self::expected(self::LARGE_FORMAT) => false,
// ],
'Large format to Belgium' => $this->getDefaultAddress(AbstractConsignment::CC_BE) + [
self::CUSTOMS_DECLARATION => $this->getDefaultCustomsDeclaration(
'Large format to Belgium' => $instance->getDefaultAddress(AbstractConsignment::CC_BE) + [
self::CUSTOMS_DECLARATION => $instance->getDefaultCustomsDeclaration(
AbstractConsignment::CC_BE
),
self::LARGE_FORMAT => true,
Expand All @@ -123,12 +125,13 @@ public function provideLargeFormatData(): array
/**
* @throws \Exception
*/
public function provideShipmentOptionsWithPickupData(): array
public static function provideShipmentOptionsWithPickupData(): array
{
return $this->createConsignmentProviderDataset(
$instance = new self();
return $instance->createConsignmentProviderDataset(
[
'Pickup with shipment options PostNL' => [
self::DELIVERY_DATE => $this->generateDeliveryDate(),
self::DELIVERY_DATE => $instance->generateDeliveryDate(),
self::ONLY_RECIPIENT => true,
self::RETURN => true,
self::SIGNATURE => true,
Expand All @@ -145,9 +148,9 @@ public function provideShipmentOptionsWithPickupData(): array
* @return array
* @throws \Exception
*/
public function provideMailboxData(): array
public static function provideMailboxData(): array
{
return $this->createConsignmentProviderDataset([
return (new self())->createConsignmentProviderDataset([
'Mailbox shipment' => [
self::PACKAGE_TYPE => AbstractConsignment::PACKAGE_TYPE_MAILBOX,
],
Expand All @@ -172,9 +175,9 @@ public function provideMailboxData(): array
* @return array
* @throws \Exception
*/
public function provideInsuranceData(): array
public static function provideInsuranceData(): array
{
return $this->createConsignmentProviderDataset([
return (new self())->createConsignmentProviderDataset([
'EUR 250' => [
self::INSURANCE => 250,
self::expected(self::ONLY_RECIPIENT) => true,
Expand All @@ -198,11 +201,12 @@ public function testInsurance(array $testData): void
* @return array
* @throws \Exception
*/
public function providePickupLocationData(): array
public static function providePickupLocationData(): array
{
return $this->createConsignmentProviderDataset([
$instance = new self();
return $instance->createConsignmentProviderDataset([
'Pickup location' => [
self::DELIVERY_DATE => $this->generateDeliveryDate(),
self::DELIVERY_DATE => $instance->generateDeliveryDate(),
self::DELIVERY_TYPE => AbstractConsignment::DELIVERY_TYPE_PICKUP,
self::PICKUP_CITY => 'Hoofddorp',
self::PICKUP_COUNTRY => AbstractConsignment::CC_NL,
Expand All @@ -219,15 +223,16 @@ public function providePickupLocationData(): array
* @return array
* @throws \Exception
*/
public function provideReferenceIdentifierData(): array
public static function provideReferenceIdentifierData(): array
{
return $this->createConsignmentProviderDataset([
$instance = new self();
return $instance->createConsignmentProviderDataset([
// 'normal consignment with reference id' => [
// [self::REFERENCE_IDENTIFIER => $this->generateTimestamp() . '_normal_consignment'],
// ],
'two consignments with reference identifiers' => [
[self::REFERENCE_IDENTIFIER => $this->generateTimestamp() . '_2_1'],
[self::REFERENCE_IDENTIFIER => $this->generateTimestamp() . '_2_2'],
[self::REFERENCE_IDENTIFIER => $instance->generateTimestamp() . '_2_1'],
[self::REFERENCE_IDENTIFIER => $instance->generateTimestamp() . '_2_2'],
],
]);
}
Expand Down
Loading