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 to be able to finish it successfully #56

Conversation

alquerci
Copy link

@alquerci alquerci commented Sep 8, 2018

What provides this patch?

  • Activates tests execution
  • Fix side effect of testConnectionInformationDecoded as it switch the current connection but never restore.

Status before this patch

PHP 5.3

  • Doctrine_Record_Synchronize_TestCase

    • testSynchronizeAfterSaveRecord
    • testSynchronizeAfterAddRecord

    Issue: When the order is not explicit then you must not expect that relationships are ordered by the primary key.

  • Search behavior regarding record generator

PHP > 5.3

  • I18n and Search behavior regarding record generator

PHP 7

  • Doctrine_Record_Filter_TestCase
    • testCompoundFilterSupportsAccessingRelatedComponentProperties

PHP all

  • Doctrine_Ticket_1783_TestCase

    • testValidateLargeIntegers
  • Doctrine_Query_MultiJoin_TestCase

    • testMultipleOneToManyFetching
    • testMultipleOneToManyFetching2

    Issue: When the order is not explicit then you must not expect that relationships are ordered by the primary key.

  • Doctrine_Record_FromArray_TestCase

    • testFromArrayAfterSaveRecord

    Issue: When the order is not explicit then you must not expect that relationships are ordered by the primary key.

Status after this patch

All tests passes.

cc @j0k3r @GromNaN

@GromNaN GromNaN requested a review from j0k3r September 9, 2018 12:04
@j0k3r
Copy link

j0k3r commented Sep 10, 2018

Could you check why tests are failing?
Thanks

@alquerci
Copy link
Author

alquerci commented Sep 10, 2018

@j0k3r From the doctrine1 documentation:

It is not uncommon for the test suite to have fails that we are aware of. Often Doctrine will have test cases for bugs or enhancement requests that cannot be committed until later versions. Or we simply don’t have a fix for the issue yet and the test remains failing. You can ask on the mailing list or in IRC for how many fails should be expected in each version of Doctrine.

All its failures are currently know as failed. For sure we need to take time to fix them.
I estimate in 4 pull request to fix them all.

As I see there is already one #31

@alquerci
Copy link
Author

alquerci commented Oct 29, 2018

Hello @thePanz @mkopinsky

Can you take a look at this patch?

tests/ManagerTestCase.php Outdated Show resolved Hide resolved
tests/ManagerTestCase.php Outdated Show resolved Hide resolved
@thePanz
Copy link
Member

thePanz commented Oct 31, 2018

@alquerci added a couple of comments, can you rebase your PR to the new repository fos1/doctrine1?

@alquerci alquerci force-pushed the fix-tests-to-be-able-to-finish-it-without-a-fatal-error branch from a09379f to c7c6c9a Compare October 31, 2018 16:28
@alquerci
Copy link
Author

@thePanz Code review fixed and the branch is rebased.

Also I add tests/tmp to .gitignore and finish to revert this commit 826bc90.

@alquerci
Copy link
Author

alquerci commented Apr 6, 2019

@thePanz What left to do on this PR in order to be merged?

Do we really need to fix all issues on it?

@thePanz
Copy link
Member

thePanz commented Apr 10, 2019

@alquerci I merged some other PRs, can we try to rebase this and check the status again?
Thank you for your patience :)

@alquerci alquerci force-pushed the fix-tests-to-be-able-to-finish-it-without-a-fatal-error branch from c7c6c9a to 1c47abd Compare April 12, 2019 06:53
@alquerci
Copy link
Author

alquerci commented Apr 12, 2019

@thePanz Rebased but it seems that it miss the Travis integration.

Until: https://travis-ci.org/alquerci/doctrine1/builds/519115319

@thePanz
Copy link
Member

thePanz commented Apr 12, 2019

@alquerci TravisCI has been re-enabled, can you push again to trigger the CI to run?
The first tests are green now (php 5.4 and php 5.5) for: https://travis-ci.org/FriendsOfSymfony1/doctrine1/builds/519309877

@alquerci alquerci force-pushed the fix-tests-to-be-able-to-finish-it-without-a-fatal-error branch from 1c47abd to 55526b8 Compare April 12, 2019 18:48
@alquerci
Copy link
Author

The first tests are green now

Just because nothing as been executed.
@thePanz Look at this one: https://travis-ci.org/FriendsOfSymfony1/doctrine1/builds/519393560

@thePanz
Copy link
Member

thePanz commented Apr 14, 2019

@alquerci damn, saw them after writing that message

@j0k3r j0k3r removed their request for review July 16, 2019 07:02
@alquerci
Copy link
Author

alquerci commented Aug 27, 2019

Hello @thePanz

As you said me you took this PR. Did you done any progress or I can resume my work on it in order to have a green status ?

The goal will not to fix all failed test here where for some of there there will be mark as "known failed".

I will be able to work on it during week-ends.

@thePanz
Copy link
Member

thePanz commented Aug 28, 2019

@alquerci unfortunately I had no more time to continue working on this. feel free to take over the MR

@alquerci
Copy link
Author

@thePanz Thank you for your fast answer.

@alquerci alquerci force-pushed the fix-tests-to-be-able-to-finish-it-without-a-fatal-error branch 7 times, most recently from 1b42b43 to 63363f3 Compare September 1, 2019 23:14
Copy link
Collaborator

@Tybaze Tybaze left a comment

Choose a reason for hiding this comment

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

Checked each commit - good job
Run with all env everything is ok

.gitignore Show resolved Hide resolved
.gitignore Show resolved Hide resolved
tests/ManagerTestCase.php Outdated Show resolved Hide resolved
tests/BaseTestCase.php Outdated Show resolved Hide resolved
lib/Doctrine/Record/Filter/Compound.php Show resolved Hide resolved
tests/BaseTestCase.php Outdated Show resolved Hide resolved
@alquerci alquerci force-pushed the fix-tests-to-be-able-to-finish-it-without-a-fatal-error branch 2 times, most recently from c67b544 to 7376e97 Compare July 25, 2022 02:09
tests/DoctrineTest/Doctrine_UnitTestCase.php Outdated Show resolved Hide resolved
lib/Doctrine/Record/Filter/Compound.php Show resolved Hide resolved
lib/Doctrine/Record/Filter/Standard.php Show resolved Hide resolved
lib/Doctrine/Record/Filter/Standard.php Show resolved Hide resolved
lib/Doctrine/Record/Filter/Compound.php Show resolved Hide resolved
lib/Doctrine/Record/Filter/Compound.php Outdated Show resolved Hide resolved
@alquerci alquerci force-pushed the fix-tests-to-be-able-to-finish-it-without-a-fatal-error branch from 3c1a6cf to b5b0ce1 Compare August 11, 2022 13:58
Copy link
Member

@thePanz thePanz left a comment

Choose a reason for hiding this comment

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

We're almost there 👍

Just one small change

lib/Doctrine/Record/Filter.php Outdated Show resolved Hide resolved
@thePanz
Copy link
Member

thePanz commented Oct 5, 2022

LGTM @alquerci , woudl you squash some of the commits to have a cleaner git history? wdyt?

@alquerci alquerci force-pushed the fix-tests-to-be-able-to-finish-it-without-a-fatal-error branch from ad477d1 to da740ae Compare October 5, 2022 10:02
@alquerci
Copy link
Author

alquerci commented Oct 5, 2022

Ready to merge.

@thePanz thePanz merged commit ae10c17 into FriendsOfSymfony1:master Oct 5, 2022
@thePanz
Copy link
Member

thePanz commented Oct 5, 2022

Merged @alquerci (wow, 4 years and 1 month after opening it) 🙈

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.

4 participants