[make:entity] Fix error when API-Platform is installed.

[MichaelBrauner]

I'll try to get the testsuite running. Then I will also write a test for that.

[weaverryan]

It'd be great to have a test to "cover" the error. However, the fact that the tests DO pass with this change (the @dev tests are currently broken unrelate to this PR, I believe) and that the change is a good one, I would give this a +1

[MichaelBrauner]

Thank you Ryan.

I tried the last days to get the tests running. Marbulk helped me a lot - because I never wrote tests before. Actually thats something I want to watch next on symfonycasts ;) .

But I can't get the MakeEntityTest running.
The symfony applications in tmp/cache created for every dataset do not have a DATABASE_URL inside the .env files.
So I get an error, when it should be replaced with the TEST_DATABASE_DSN inside the phpunit.xml.

Hm. Are this the tests wich are currently broken - as u said before?

Because when I run all tests of maker-bundle I can see that there are test succeeding.

[ghost]

@weaverryan Any help for the new contributor from the maintainer? I think Symfony is all about contributors.

[MichaelBrauner]

Hm, I don't really see me as a contributer yet. I just found a bug and wrote a few lines to fix that.

I also tried 3 days long to make those tests work. But I am completly new to testing.

I also have a fulltime-job and for these 3 days of trying to make the tests work, I stopped working on my side projects.
And also in case these tests are failing because of something else (not related to my system) - I don't want to fix also this :D.

[mpiot]

Tests fail seem unrelated to the PR. To trigger the actual bug, you should update or create a duplicate of it:

File: https://github.com/symfony/maker-bundle/blob/main/tests/Maker/MakeEntityTest.php

        yield 'entity_new' => [MakerTestDetails::createTest(
            $this->getMakerInstance(MakeEntity::class),
            [
                // entity class name
                'User',
                // add not additional fields
                '',
            ])
            ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeEntity')
            ->configureDatabase()
            ->updateSchemaAfterCommand(),
        ];

This test simulate the creation of a basic new Entity, without any fields. The wizard do not ask about Broadcasting or APiPlatform (because not simulated as installed).

In this test:

        yield 'entity_new_api_resource' => [MakerTestDetails::createTest(
            $this->getMakerInstance(MakeEntity::class),
            [
                // entity class name
                'User',
                // Mark the entity as an API Platform resource
                'y',
                // add not additional fields
                '',
            ])
            ->addExtraDependencies('api')
            ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeEntity')
            ->configureDatabase()
            ->updateSchemaAfterCommand()
            ->assert(function (string $output, string $directory) {
                $this->assertFileExists($directory.'/src/Entity/User.php');

                $content = file_get_contents($directory.'/src/Entity/User.php');
                $this->assertStringContainsString('use ApiPlatform\Core\Annotation\ApiResource;', $content);
                $this->assertStringContainsString(\PHP_VERSION_ID >= 80000 ? '#[ApiResource]' : '@ApiResource', $content);
            }),
        ];

You have the same has before but with api platform, then in :

        yield 'entity_new_broadcast' => [MakerTestDetails::createTest(
            $this->getMakerInstance(MakeEntity::class),
            [
                // entity class name
                'User',
                // Mark the entity as broadcasted
                'y',
                // add not additional fields
                '',
            ])
            ->setRequiredPhpVersion(70200)
            ->addExtraDependencies('ux-turbo-mercure')
            ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeEntity')
            ->configureDatabase()
            ->addReplacement(
                '.env',
                'https://127.0.0.1:8000/.well-known/mercure',
                'http://127.0.0.1:1337/.well-known/mercure'
            )
            ->updateSchemaAfterCommand()
            ->assert(function (string $output, string $directory) {
                $this->assertFileExists($directory.'/src/Entity/User.php');

                $content = file_get_contents($directory.'/src/Entity/User.php');
                $this->assertStringContainsString('use Symfony\UX\Turbo\Attribute\Broadcast;', $content);
                $this->assertStringContainsString(\PHP_VERSION_ID >= 80000 ? '#[Broadcast]' : '@Broadcast', $content);
            }),
        ];

You create a basic one with a broadcast.

In your case you want to create one new, simulating APiPlatform and Broadcat installed, but say "n" to their usage, it give something like:

        yield 'entity_new_with_api_and_broadcast_dependencies' => [MakerTestDetails::createTest(
            $this->getMakerInstance(MakeEntity::class),
            [
                // entity class name
                'User',
                // Mark the entity as not an API Platform resource
                'n',
                // Mark the entity as not broadcasted
                'n',
                // add not additional fields
                '',
            ])
            ->setRequiredPhpVersion(70200)
            ->addExtraDependencies('api')
            ->addExtraDependencies('ux-turbo-mercure')
            ->setFixtureFilesPath(__DIR__.'/../fixtures/MakeEntity')
            ->configureDatabase()
            ->updateSchemaAfterCommand(),
        ];

[weaverryan]

@mpiot if you’re willing and able, you could create another PR with the failing test. Then I could merge in both PR’s (your pr with the failing test and this fix). I’m just trying to see what we can do to move this forward given that we have several people trying to help!

cheers!

[mpiot]

@weaverryan @MichaelBrauner just implement the missing test. I prefer help him to finish his PR (more interesting, I think). But if needed, I'll do that :)

[MichaelBrauner]

I did implement the test. I also tried it on my lokal project. It definetly works and fixes the bug.

When the failing tests are not related to my PR, how can I then finish this PR?

[mpiot]

@MichaelBrauner Just need to wait a maintener approve the workflow running and approve your change (if not problems in code / tests)

[weaverryan]

Awesome! Thanks to both @MichaelBrauner and @mpiot for getting this done :)