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

SchemaValidator: Changing mapping of BIGINT to string|int #11399

Merged
merged 1 commit into from
Mar 28, 2024

Conversation

ThomasLandauer
Copy link
Contributor

Closes #11377
Follow-up of #11396

I just copied the const, so it can be deleted again easily, once DBAL 3 is no longer supported.

  • How can I check DBAL 3 in the test?
  • Besides that, is the test enough or should I test more scenarios (positive and negative)?

@greg0ire
Copy link
Member

greg0ire commented Mar 23, 2024

How can I check DBAL 3 in the test

Same way you did in the code.

Besides that, is the test enough or should I test more scenarios (positive and negative)?

I think a test testing what happens when using just int would be nice to have.

Also, it seems there are CI jobs failing. Please take a look at this guide for more on how to handle those.

@ThomasLandauer
Copy link
Contributor Author

ThomasLandauer commented Mar 23, 2024

OK, I also added another test for just string.

What I don't understand: composer install gave me DBAL 4. So where/when is the situation with DBAL 3 ever tested? If the line if (method_exists(BigIntType::class, 'getName')) is not doing what we hope it does (e.g. is always false), then the test will not detect it (since it uses the same logic as the code).

@greg0ire
Copy link
Member

This step allows us to test with DBAL 3:

- name: "Require specific DBAL version"
run: "composer require doctrine/dbal ^${{ matrix.dbal-version }} --no-update"
if: "${{ matrix.dbal-version != 'default' }}"

thanks to this:

dbal-version:
- "default"
- "3.7"

If the version detection code is wrong, then the test should fail because it will expect the wrong thing to happen.

@greg0ire
Copy link
Member

Pleas squash your commits.

@ThomasLandauer
Copy link
Contributor Author

OK, done :-)

@greg0ire greg0ire added the Bug label Mar 28, 2024
@greg0ire greg0ire merged commit cbb6c89 into doctrine:3.1.x Mar 28, 2024
64 checks passed
@greg0ire greg0ire added this to the 3.1.2 milestone Mar 28, 2024
@greg0ire
Copy link
Member

Thanks @ThomasLandauer !

@ThomasLandauer ThomasLandauer deleted the issue-11377 branch March 28, 2024 21:34
@derrabus derrabus mentioned this pull request Apr 15, 2024
derrabus added a commit to derrabus/orm that referenced this pull request Apr 15, 2024
…1377"

This reverts commit cbb6c89, reversing
changes made to 9c56071.
derrabus added a commit that referenced this pull request Apr 15, 2024
@derrabus
Copy link
Member

Thank you very much for your work on this topic, @ThomasLandauer. However, we've decided to revert your patch in favor of #11414.

derrabus added a commit that referenced this pull request Apr 15, 2024
* 3.1.x:
  Revert "Merge pull request #11399 from ThomasLandauer/issue-11377" (#11415)
  Fix BIGINT validation (#11414)
  docs: update PHP version in doc
  Fix fromMappingArray definition
  Fix templated phpdoc return type (#11407)
  [Documentation] Merging "Query Result Formats" with "Hydration Modes"
  SchemaValidator: Changing mapping of BIGINT to string|int
  Fix psalm errors: remove override of template type
  Update dql-doctrine-query-language.rst
  Adding `NonUniqueResultException`
  [Documentation] Query Result Formats
derrabus added a commit that referenced this pull request Apr 15, 2024
* 3.2.x:
  Revert "Merge pull request #11399 from ThomasLandauer/issue-11377" (#11415)
  Fix BIGINT validation (#11414)
  docs: update PHP version in doc
  Fix fromMappingArray definition
  Fix templated phpdoc return type (#11407)
  [Documentation] Merging "Query Result Formats" with "Hydration Modes"
  SchemaValidator: Changing mapping of BIGINT to string|int
  Fix psalm errors: remove override of template type
  Update dql-doctrine-query-language.rst
  Adding `NonUniqueResultException`
  [Documentation] Query Result Formats
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.

SchemaValidator is complaining about BIGINT with PHP typehint int
4 participants