Implement the changes made in symfony-cmf/routing-auto#91

[damienflament]

This PR:

• fixes the .gitignore file in order to REALLY ignore the cache/logs directories of the testing app,
• adds a Composer script to run the tests as configured in the .travis.yml file (can be run using composer test)
• implements the new AdapterInterface::compareAutoRouteLocale method added by symfony-cmf/routing-auto#91.

[damienflament]

Can you re-run the checks when #203 is merged ? The tests should pass. Thanks !

[dantleech]

Can we add a use-case test in the AutoRouteListenerTest?

I have just tried

    public function testMultilangArticleNoSharding()
    {
        $article = new NoShardingArticle();
        $article->path = '/test/article-1';
        $article->title = 'Weekend';
        $this->getDm()->persist($article);
        $this->getDm()->bindTranslation($article, 'fr');

        $article = new NoShardingArticle();
        $article->path = '/test/article-2';
        $article->title = 'Weekend';
        $this->getDm()->persist($article);
        $this->getDm()->bindTranslation($article, 'en');

        $this->getDm()->flush();
        $this->getDm()->clear();

        $route = $this->getDm()->find(null, 'test/auto-route/articles/weekend');
        $this->assertNotNull($route);

        $route = $this->getDm()->find(null, 'test/auto-route/articles/weekend-1');
        $this->assertNotNull($route);
    }

Symfony\Cmf\Bundle\RoutingAutoBundle\Tests\Resources\Document\NoShardingArticle:
    definitions: 
        view:
            uri_schema: /articles/{article_title}
            defaults:
                _type: article_view
    token_providers:
        article_title: [content_method, { method: getTitle } ]
    conflict_resolver: [auto_increment, { }]

But it passes both on master and on your branch, so I guess I'm not testing it right :-\

Thanks for all your work on this by the way :)

[damienflament]

@dantleech : Your test creates two documents. As their auto routes are created using two different UriContextCollection, the conflict is resolved.

To reveal the bug, you have to create two translations of the same document.

I added the test.

Note: Adding a document class in order to use a different routing-auto configuration is tedious. Loading a configuration file within the test method might be easier !

[dantleech]

tedious

Indeed, we could build the config/class each time, but you also pay the price of adding more noise to the test case. Sthg such as Behat might be better for this type of test.

btw. this test needs to be refactored as a general acceptance test for any driver - which is a pre-requisite for developing f.e. an ORM driver.

Also at some point it would make sense tomove the ODM driver (and this acceptence test) to the component, having it here was a mistake.

to reveal the bug...

Thanks! Will give this a final look tomorrow at the latest and we can merge :)

[damienflament]

@dantleech : Sure. It is testing using the master branch of routing-auto. It is just revealing the bug fixed by symfony-cmf/routing-auto#91. Tests pass using conflict-resolving branch in my repo.

[dantleech]

Looks like we will need to change the routing-auto dep to dev-master /cc @dbu

[dbu]

i just tagged routing-auto 2.0.0-RC2 so you can now require ^2.0.0-RC2.

[damienflament]

No needs to explicitly require ^2.0.0-RC2 as the minimum-stability is set to RC.

[damienflament]

Why testing with the lowest version for PHP 5.6 and not for other environments ?

I don't understand well the Travis configuration file. The Perl "hack" used to modify the Composer configuration make me think that it needs some refactoring because the composer config command do it well.

[dantleech]

Why testing with the lowest version for PHP 5.6 and not for other environments

To reduce build time IIRC @wouterj @dbu

The Perl "hack" used to modify the Composer configuration

Presumably influenced/copied from Symfony, maybe @wouterj or @dbu can help.

[damienflament]

I read about the --prefer-lowest Composer flag. Its usage seams useful. But that means this PR make the routing-auto-bundle no longer supports routing-auto 2.0.0-RC1.

Should we require 2.0.0-RC2 as @dbu said and add this new minimum requirement to the Changelog ?

[dantleech]

Yes - the bundle will be incompatible with RC1, or?

On 18 June 2017 22:12:53 BST, Damien Flament notifications@github.com wrote:

I read about the --prefer-lowest Composer flag. Its usage seams useful. But that means this PR make the routing-auto-bundle no longer supports routing-auto 2.0.0-RC1.

Should we require 2.0.0-RC2 as @dbu said and add this new minimum requirement to the Changelog ?

-- You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub: https://github.com/symfony-cmf/routing-auto-bundle/pull/202#issuecomment-309303306

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

[dantleech]

Thanks again :+1:

[dbu]

thanks a lot @damienflament! to answer some of your questions:

Why testing with the lowest version for PHP 5.6 and not for other environments ?

we don't need to test every combination of php version and symfony version. its enough to test all important symfony versions once and all php versions once with the latest symfony. we only care about testing the php support of our bundle, not of the dependencies. so as dan said, its about reducing the load on travis-ci (we are limited in the number of parallel builds, so having too many combinations to build also punishes us directly)

Should we require 2.0.0-RC2 as @dbu said and add this new minimum requirement to the Changelog ?

changes in minimum requirement don't need to go into the changelog imho, apart maybe from symfony major version or such things. the changelog is about code changes. bumping which RC we need at least is not a relevant change for the consumer.

and yes, --prefer-lowest is why i proposed to bump it to ^2.0.0-RC2. also to avoid somebody installing the newest bundle but with RC1 of routing-auto and then getting errors.

[damienflament]

I was not aware of the --prefer-lowest option and its usefulness.

Thanks for your explanations !