symfony-cmf / cmf-sandbox

Base project for trying CMF components integration
http://cmf.symfony.com
Other
367 stars 140 forks source link

Fix master branch and its travis tests #397

Closed ElectricMaxxx closed 6 years ago

ElectricMaxxx commented 6 years ago

I just wanna try to fix the tests on master.

dbu commented 6 years ago

@nicolas-grekas would you have an idea why the phpunit-simple crashes the way it does on travis-ci with everything but php 7.1?

nicolas-grekas commented 6 years ago

No idea, maybe be a memory limit that travis has in place? Can you make it work locally on 7.0?

ElectricMaxxx commented 6 years ago

The difference is not the php version. I the composer-instal flag we use in travis.

ElectricMaxxx commented 6 years ago

but i'll try it at home tonight

SenseException commented 6 years ago

AdminTest::testAdmin() seems to have a lot of work. What's tested there?

ElectricMaxxx commented 6 years ago

Yes, the test is a little bit longer, as it calls each admin page we have in the sandbox and checks wheter it returns a 200 or not.

SenseException commented 6 years ago

On skipping that test: Call to undefined method Doctrine\ODM\PHPCR\Event\PreUpdateEventArgs::getDocument().

ElectricMaxxx commented 6 years ago

Interesting. Started to divide the test trough a dataprovider today, but did not get it working yet.

SenseException commented 6 years ago

I've also tested AdminTest isolated, ignoring the other ones, which makes the build go green. But the test triggers a warning.

There was 1 warning:

1) Warning
The data provider specified for Tests\Functional\AdminTest::testAdmin is invalid.
Data set #0 is invalid.
SenseException commented 6 years ago

@ElectricMaxxx Can you please try the following code for the getAdmin data provider in your AdminTest?

public function getAdmin()
{
    $pool = $this->getContainer()->get('sonata.admin.pool');
    $adminGroups = $pool->getAdminGroups();

    $admins = function() use ($adminGroups, $pool) {
        foreach (array_keys($adminGroups) as $adminName) {
        yield $pool->getAdminsByGroup($adminName);
        }
    };

    return $admins();
}

At least to see if this will make the build run until the end.

ElectricMaxxx commented 6 years ago

@SenseException you can do aa PR against my branch, pleaae? I am currently mobile only.

SenseException commented 6 years ago

I'll see if I can do it at late evening.

SenseException commented 6 years ago

@ElectricMaxxx I've created #398.

ElectricMaxxx commented 6 years ago

This one should fix it: https://github.com/sonata-project/cache/pull/103

ElectricMaxxx commented 6 years ago

@dbu having a look at this "bug" is really wired. On the one hand caching should never have worked, as there was never a method call getObject(), but this does not matter. A thing that makes me crazy is: why do we trigger a pre update event when calling a admin route by a GET request? Is there something else wrong on our side?

ElectricMaxxx commented 6 years ago

aannd ... the AutoRouteListener does a flush again (what looks strange, the check) and preUpdate is triggerd on DoctrinePHPCRODMListnerContainerAware There must be an issue on this path.

dbu commented 6 years ago

yeah, we should not be generating anything at that point. and i don't know why we would want sonata cache active by default, there is no active cache invalidation anywhere so it should not be listening in, imho. i wonder if thats a default that changed on sonata side somewhere.

ElectricMaxxx commented 6 years ago

Seems as i come closer: https://travis-ci.org/symfony-cmf/cmf-sandbox/jobs/333956339 The issue is the php version in sonata-project/cache. So we can either have an issue with a broken listener in cache (<v1.1.1) or we can have an issue with php version ... So i bumped php versions for now.

ElectricMaxxx commented 6 years ago

yea travis became green. So there was one chance only: going on php 7.1 and drop lower.

ElectricMaxxx commented 6 years ago

No clue what the problem of plattform is, the link seems to be fine