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

tests: automatically create cascade persist permutations #666

Draft
wants to merge 5 commits into
base: 2.x
Choose a base branch
from

Conversation

nikophil
Copy link
Member

@nikophil nikophil commented Jul 3, 2024

fixes #597

This PR introduces a way to automatically create all permutations for "cascade persist" relationships.

By using a combination of the trait WithEntityRelationShip, and the attribute UsingRelationShips, a data provider is automatically created with all possible combinations of the "cascade persist" and "standard" associations.

ex:

final class EntityRelationshipTest extends KernelTestCase
{
    use WithEntityRelationShip, Factories, ResetDatabase;

    // a matrix with all relationships will be created. Here, for example, there will be 4 permutations:
    // [`Contact::$category` => "cascade", `Category::$contacts` => "cascade"]
    // [`Contact::$category` => "standard", `Category::$contacts` => "cascade"]
    // [`Contact::$category` => "cascade", `Category::$contacts` => "standard"]
    // [`Contact::$category` => "standard", `Category::$contacts` => "standard"]
    #[UsingRelationShips(Contact::class, ['category'])]
    #[UsingRelationShips(Category::class, ['contacts'])]
    /**
     * @test
     * @dataProvider provideCascadeRelationshipsCombination
     */
    public function many_to_one(): void
    {
        // add some test which used `Contact::$category` and `Category::$contacts`
    }
}

If we're OK on this implementation, I think I can remove all StandardXXX and CascadeXXX classes!

Here are some thoughts about this work:

  • We must add the data provider on each method using the attribute UsingRelationShips. I don't think it is possible to automatically add it, but I've added a check which enforces to add the provider if the attribute is used
  • It is currently not possible to have another data provider. But for now, the tests about "orm relationships" do not use data providers, so it's good enough
  • the trait WithEntityRelationShip MUST be before Factories trait. I have to find a way to enforce that. It is a problem of order in the @before hooks and this is why I decided to propose this change in PHPUnit
  • I do think we should test ALL these permutations (I mean, the full matrix, with full cases), we may find some weird edge cases

@nikophil nikophil marked this pull request as draft July 3, 2024 17:38
/**
* @return iterable<list<DoctrineCascadeRelationshipMetadata>>
*/
private static function provideCascadeRelationshipsCombination(string $methodName): iterable
Copy link
Member Author

Choose a reason for hiding this comment

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

I need to add some tests for this stuff as well

@nikophil nikophil force-pushed the tests/automatic-test-of-cascade-persist-combination branch 6 times, most recently from 0b9dfbd to d1794c6 Compare December 11, 2024 17:41
@nikophil nikophil force-pushed the tests/automatic-test-of-cascade-persist-combination branch from d1794c6 to 9cd3f3d Compare December 11, 2024 18:28
@nikophil nikophil force-pushed the tests/automatic-test-of-cascade-persist-combination branch from 9cd3f3d to dac8bbe Compare December 11, 2024 18:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

[2.x] Automatically test all "cascade" combinations in tests
2 participants