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.

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 tests #350

Merged
merged 1 commit into from
Jan 30, 2024
Merged

Fix tests #350

merged 1 commit into from
Jan 30, 2024

Conversation

ker0x
Copy link
Contributor

@ker0x ker0x commented Jan 26, 2024

No description provided.

@ker0x ker0x force-pushed the bugfix/tests branch 3 times, most recently from ff4d82c to b3d5485 Compare January 26, 2024 10:11
@norkunas
Copy link
Member

I think instead of setting SYMFONY_DEPRECATIONS_HELPER: 'max[self]=3&max[indirect]=1' we could use deprecations baseline file, would be easier

@@ -0,0 +1,41 @@
[
Copy link
Member

Choose a reason for hiding this comment

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

From my experience this file can change randomly when running on different devices.
I suggest this: https://symfony.com/doc/current/components/phpunit_bridge.html#ignoring-deprecations

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The ignoreFile option is only available since Symfony 6.1!

Copy link
Member

Choose a reason for hiding this comment

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

Ah, sad :(

phpunit.xml.dist Outdated Show resolved Hide resolved
phpunit.xml.dist Outdated Show resolved Hide resolved
Comment on lines 18 to 16
"message": "Since symfony/framework-bundle 5.1: Not setting the \"framework.router.utf8\" configuration option is deprecated, it will default to \"true\" in version 6.0.",
"count": 41
},
{
"message": "Since symfony/framework-bundle 6.4: Not setting the \"framework.handle_all_throwables\" config option is deprecated. It will default to \"true\" in 7.0.",
"count": 82
},
{
"message": "Since symfony/framework-bundle 6.4: Not setting the \"framework.php_errors.log\" config option is deprecated. It will default to \"true\" in 7.0.",
"count": 82
},
{
"message": "Since symfony/framework-bundle 6.4: Enabling the integration of Doctrine annotations is deprecated. Set the \"framework.annotations.enabled\" config option to false.",
"count": 41
},
{
"message": "Since symfony/framework-bundle 6.4: The \"annotation_reader\" service alias is deprecated without replacement. It is being referenced by the \"routing.loader.attribute\" service.",
"count": 41
},
{
"message": "Since symfony/framework-bundle 6.4: The \"annotation_reader\" service alias is deprecated without replacement. It is being referenced by the \"doctrine.orm.metadata.annotation_reader\" alias.",
"count": 5
}
Copy link
Member

Choose a reason for hiding this comment

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

Btw these should be fixed, not ignored ;)

@ker0x ker0x force-pushed the bugfix/tests branch 2 times, most recently from 13c9e58 to 0e88735 Compare January 26, 2024 13:25
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@ker0x ker0x force-pushed the bugfix/tests branch 14 times, most recently from 8c4725c to 090eef4 Compare January 29, 2024 09:23
uses: actions/checkout@4

- name: "Configure Symfony"
run: composer config extra.symfony.require "${{ matrix.sf_version }}"

- name: "Install Composer dependencies"
uses: "ramsey/composer-install@v2"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
uses: "ramsey/composer-install@v2"
uses: "ramsey/composer-install@v2"
env:
SYMFONY_REQUIRE: ${{ matrix.sf_version }}

Comment on lines 95 to 96
uses: actions/checkout@4

- name: "Configure Symfony"
run: composer config extra.symfony.require "${{ matrix.sf_version }}"

- name: "Install Composer dependencies"
Copy link
Member

@norkunas norkunas Jan 29, 2024

Choose a reason for hiding this comment

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

Suggested change
uses: actions/checkout@4
- name: "Configure Symfony"
run: composer config extra.symfony.require "${{ matrix.sf_version }}"
- name: "Install Composer dependencies"
uses: actions/checkout@v4
- name: "Install Composer dependencies"

.editorconfig Outdated
Comment on lines 11 to 15
[*.json]
indent_size = 2

[.github/**/*.yml]
indent_size = 2
Copy link
Member

Choose a reason for hiding this comment

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

not sure what's the point of these changes if PR targets at fixing tests :) only hardens the review

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Every time I modify the composer.json or the ci.yml my IDE indents the additions with 4 spaces as defined in the .editorconfig whereas they are indented with 2 spaces, these 2 lines save me from having to correct the indentation manually

Copy link
Member

Choose a reason for hiding this comment

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

Im ok with these changes but they pollute the PR, could you split into separate PR later?

Comment on lines 17 to 46
{
"location": "Bazinga\\GeocoderBundle\\Tests\\Functional\\GeocoderListenerTest::testPersistForProperty",
"message": "Since doctrine/doctrine-bundle 2.11: Not setting \"enable_lazy_ghost_objects\" to true is deprecated.",
"count": 1
},
{
"location": "Bazinga\\GeocoderBundle\\Tests\\Functional\\GeocoderListenerTest::testPersistForGetter",
"message": "Since doctrine/doctrine-bundle 2.11: Not setting \"enable_lazy_ghost_objects\" to true is deprecated.",
"count": 1
},
{
"location": "Bazinga\\GeocoderBundle\\Tests\\Functional\\GeocoderListenerTest::testPersistForInvalidGetter",
"message": "Since doctrine/doctrine-bundle 2.11: Not setting \"enable_lazy_ghost_objects\" to true is deprecated.",
"count": 1
},
{
"location": "Bazinga\\GeocoderBundle\\Tests\\Functional\\GeocoderListenerTest::testPersistForEmptyProperty",
"message": "Since doctrine/doctrine-bundle 2.11: Not setting \"enable_lazy_ghost_objects\" to true is deprecated.",
"count": 1
},
{
"location": "Bazinga\\GeocoderBundle\\Tests\\Functional\\GeocoderListenerTest::testDoesNotGeocodeIfAddressNotChanged",
"message": "Since doctrine/doctrine-bundle 2.11: Not setting \"enable_lazy_ghost_objects\" to true is deprecated.",
"count": 1
},
Copy link
Member

Choose a reason for hiding this comment

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

these could be resolved with:

if (method_exists(Configuration::class, 'setLazyGhostObjectEnabled') && Kernel::VERSION_ID >= 60100) {
    $container->prependExtensionConfig('doctrine', [
        'orm' => [
            'enable_lazy_ghost_objects' => true,
        ],
    ]);
}

@ker0x ker0x force-pushed the bugfix/tests branch 5 times, most recently from 4d3dfd0 to 3d1ace7 Compare January 29, 2024 13:41
@ker0x
Copy link
Contributor Author

ker0x commented Jan 29, 2024

@norkunas I'm sorry, but I think I'm going to give up because there are far too many deprecations to manage with version 4.4. Every time I fix one, another pops up right after it, it's hellish!

@norkunas
Copy link
Member

Sorry to hear that 😞 I'll try to complete it then

@ker0x ker0x force-pushed the bugfix/tests branch 8 times, most recently from 94a02d1 to 44663a7 Compare January 30, 2024 08:32
@ker0x
Copy link
Contributor Author

ker0x commented Jan 30, 2024

@norkunas Holy Moly, tests are green 🥳 🎉 ! https://github.com/ker0x/geocoder-php-geocoder-bundle/actions/runs/7708347743

Copy link
Member

@norkunas norkunas left a comment

Choose a reason for hiding this comment

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

I'm merging as is, but we should probably resolve some deprecations in follow up PR's because there are way too many of them :)

@norkunas norkunas merged commit 6c6a113 into geocoder-php:master Jan 30, 2024
14 checks passed
@norkunas
Copy link
Member

Thank you @ker0x !

@ker0x ker0x deleted the bugfix/tests branch January 30, 2024 09:01
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