Unconsistent case of properties and constructor in generated endpoints about janephp HOT 9 CLOSED

Hey @ChristopheZOL,

Thanks for reporting this issue, it's looks like a bug and your solution is correct 😉 (btw, be carefull with your example cause it does not reflect your bug since you fixed it, shouldn't it be $this->id = $id; ?)
Could you make a PR for this and add tests ? (for 4.x branch)

About tests: best way for this is to make a new fixture folder with OpenAPI definition with one endpoint with same uppercase letter and run tests, you'll have an error since you don't have expected/ folder, so just check by yourself content of the generated/ folder and copy it as expected/ 😉

from janephp.

Cool ! I'll make a PR as soon as I can. Thanks for your reactivity

from janephp.

Sorry to bother you @Korbeil, but I struggle to run tests.
I run vendor/bin/phpunit -c phpunit.xml in OpenApi directory, expecting my test to fail, as you said, but I encounter a PHP Fatal error here, due to existing tests :

PHP Fatal error:  Cannot declare class Jane\OpenApi\Tests\Expected\Endpoint\GetTest, because the name is already in use in /home/christophe/Sites/janephp/src/OpenApi/Tests/fixtures/model-in-response/expected/Endpoint/GetTest.php on line 5

It appears that 4 classes in tests are named GetTest (in the namespace Jane\OpenApi\Tests\Expected\Endpoint)

Is it normal ? Am I running PHPUnit in the wrong way ? Can I run only my test without others (seems odd to me since all tests should pass) ?

Thank you for your help

from janephp.

I suggest you running tests from JanePHP root directory (all libraries), you won't have this problem. Or you can just send a PR and Travis will run tests.

It's because we create classes in /generated folder and they often conflict with classes in /expected

from janephp.

Hello again, I managed to run all tests successfully but unfortunately, the test done does not detect my change (therefore my added test is useless because it says "OK" even if expected code is different from generated code).

This is because there is a test in JaneOpenApiResourceTest.php :

// isPhpParser4() returns true so file content check is ignored
if ($expectedFile->isFile() && !isPhpParser4()) {
    $expectedPath = $expectedFile->getRealPath();
    $actualPath = $generatedData[$expectedFile->getRelativePathname()];


    $this->assertEquals(
        file_get_contents($expectedPath),
        file_get_contents($actualPath),
        'Expected ' . $expectedPath . ' got ' . $actualPath . ' in ' . $name
    );
}

I don't understand why the fact that it's PhpParser 4 is a problem but maybe you know better than me why.
I guess this has been added to not make every test already wrote fail when moving to PhpParser4, but in this case, I think tests should have been rewritten instead of bypassing all new tests. It's more than just my added test, it seems to exclude many comparison expected vs generated in other tests :/

What do you think ? What should I do ?

from janephp.

Real problem behind that is the generated classes with PhpParser 3.x and generated classes with PhpParser 4.x are not the same, that's why I used this condition.
Let's submit your PR, and I will try to fix this after by re-generating everything with same PhpParser version as CI use 😉

from janephp.

Any news about this ? You want to submit a PR ? Or I can handle it if you prefer 😉

from janephp.

I'm sorry, like I said, I don't have so much free time to work on this. I'll make a PR today.

from janephp.

Fixed by #97 for 4.x branch

from janephp.