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

V8: Unit Test for EntityManager changes, and Bugfixes for issue #3867 #3927

Merged
merged 41 commits into from
Jun 17, 2016
Merged

V8: Unit Test for EntityManager changes, and Bugfixes for issue #3867 #3927

merged 41 commits into from
Jun 17, 2016

Conversation

Kaapiii
Copy link
Contributor

@Kaapiii Kaapiii commented Jun 9, 2016

This request adds new unit tests which cover the recent changes concerning the work on the Doctrine ORM setup and some changes to the PackageService and the Package classes.

Fixes issues from #3867:

    1. Getting package relative paths -> fixed
    1. Save path for the metadata driver configurations -> storage location was moved to the "generated_overrides"
    1. Relative paths in the saved configs -> fixed

Kaapiii added 30 commits June 4, 2016 10:04
… class which returns only the relativ path starting from the install directory
…spaces in the autoloaders "$pkgAutoloaderRegistries"
* develop:
  attribute compatibility fixes
  fixing recent problem with installation
  Close #3895
  SVGs instead of PNGs for select; making dark panel fieldless look nicer
  Fix #3894
  Make changes to the application autoload in order to allow including from a separate composer project (#3910)
  Removes toolbar gradient image, replaces with CSS equivalent. (#3907)
  CSS updates to fieldless forms; Close #3892
  changing png file type icons to svg
  new icon set
  fix add thumbnail type (#3915)
  Fieldless forms lookin so nice
  Fix working with updated core versions (#3888)
  remove unsupported attributes from source (#3889)
  Cleanup image block (#3854)
  Add support to thumbnails for SVG
* feature/entity-manager-config-factory-tests: (28 commits)
  move the storage location of package metadata for entities to "generated_overrides"
  remove all generated proxies on tearDown
  add method - which recreates the config containing the package metadata
  comment not working test
  replace concrete class type hinting with interface
  fix error in package uninstall test
  update @group annotation
  add Test which covers the uninstallation of packages
  correct wrong comment
  add test for packages without entities
  update the tests for the install method, add some src path fixes to Package
  update the tests for the install method
  update total number of installed packages
  add "Concrete" folder to core extension test package
  add new test package - with no entites
  add new test package - used for testing packages with additional namespaces in the autoloaders "$pkgAutoloaderRegistries"
  add new test package - used for testing packages with pkgAutoloaderMapCoreExtensions = true
  add additional source path to the package namespace, if property $pkgAutoloaderMapCoreExtensions is set to true
  add test for PackageService install method, add new method to Package class which returns only the relativ path starting from the install directory
  change visibility of getFileConfigORMMetadata() method
  ...
@Kaapiii
Copy link
Contributor Author

Kaapiii commented Jun 9, 2016

That's odd.
For all new tests running with php versions 5.5, 5.6 the application instance (Concrete\Core\Application\Application) is empty in the tearDown() method. So the $this->app->make('Concrete\Core\Package\PackageService'); is failing.

All tests running with php7 and hhvm and extend from \ConcreteDatabaseTestCase are failing, because the database is not configured 'Database []'. Does anybody know what might be the problem?

@aembler
Copy link
Member

aembler commented Jun 10, 2016

Thanks for taking the time to put in so many tests. Yeah, this is really weird. Running the tests locally I'm getting errors in the DatabaseTest that I don't get in the regular develop branch. I also don't get them if I just run the DatabaseTest by itself through phpstorm. It's like the Users table is empty even though the Users.xml fixture is supposed to populate its data. Maybe a new test that's running before the DatabaseTest test is causing problems with it?

@aembler
Copy link
Member

aembler commented Jun 10, 2016

Hmm, don't have time to delve further, but one of the issues is this, the testORMCacheSettings test in DatabaseServiceProviderTest appears to rebind the config object to a new test instance, without rebinding it back. Maybe that's supposed to happen automatically, but I can't run all the database tests without erroring out until I comment that test out.

I suspect something similar is happening with the other tests that are failing.

@Kaapiii
Copy link
Contributor Author

Kaapiii commented Jun 10, 2016

Thanks @aembler. I've messed up something with the config in DatabaseServiceProviderTest.php file. This test makes all following test which depend on the database connection to fail. The strange thing is, that on my local environment (tested with netbeans) the tests run successfully. I'll refactor them.

@Kaapiii
Copy link
Contributor Author

Kaapiii commented Jun 13, 2016

I could fix 2 Tests. The two other unit tests which interact with the config file and install test packages, still won't run properly with the other database related tests. So I'm going to add them in a future pull request.

…g the concrete update, add method which creates the application/src folder
…o develop

* feature/entity-manager-config-factory-updatescript:
  remove initial update script
  add method which creates the metadata of all installed packages during the concrete update, add method which creates the application/src folder
  add class to use statment
  add method to update script, which created the metadata configuration for packages
@Kaapiii
Copy link
Contributor Author

Kaapiii commented Jun 15, 2016

Fix for issues #3867 "3. Upgrading the core" is located in the Migration/Version20160420000000.php file. The metadata config for the installed packages and the application/src folder are created. I couldn't run the entire update trough because I received some "SQL constraint error" triggered by the installEntities() method.

Exception:
An exception occurred while executing 'ALTER TABLE fileattributevalues ADD CONSTRAINT FK_BBECEAA4E3111F453F7F12E4 FOREIGN KEY (fID, fvID) REFERENCES FileVersions (fID, fvID)': SQLSTATE[HY000]: General error: 1822 Failed to add the foreign key constaint. Missing index for constraint 'FK_BBECEAA4E3111F453F7F12E4' in the referenced table 'fileversions'

@aembler
Copy link
Member

aembler commented Jun 17, 2016

Yeah, I believe that...I'll look back at upgrading somewhere down the line.

@aembler aembler merged commit 58dd79b into concretecms:develop Jun 17, 2016
KorvinSzanto pushed a commit that referenced this pull request Dec 12, 2016
…#3927)

* refactor EntityManagerConfigFactory class to be testable with unit tests

* add first badge of tests

* remove not used tearDown

* add test packages

* add PackageServiceTest

* add some more tests to PackageServiceTest

* add some fixtures

* first draft of testUninstall from PackageService

* change visibility of getFileConfigORMMetadata() method

* add test for PackageService install method, add new method to Package class which returns only the relativ path starting from the install directory

* add additional source path to the package namespace, if property $pkgAutoloaderMapCoreExtensions is set to true

* add new test package - used for testing packages with pkgAutoloaderMapCoreExtensions = true

* add new test package - used for testing packages with additional namespaces in the autoloaders "$pkgAutoloaderRegistries"

* add new test package - with no entites

* add "Concrete" folder to core extension test package

* update total number of installed packages

* update the tests for the install method

* update the tests for the install method, add some src path fixes to Package

* add test for packages without entities

* correct wrong comment

* add Test which covers the uninstallation of packages

* update @group annotation

* fix error in package uninstall test

* replace concrete class type hinting with interface

* comment not working test

* add method - which recreates the config containing the package metadata

* remove all generated proxies on tearDown

* move the storage location of package metadata for entities to "generated_overrides"

* disable critical test

* remove all tests

* add EntityManagerConfigFactoryTest

* add DatabaseServiceProviderTests

* add method to update script, which created the metadata configuration for packages

* add class to use statment

* add method which creates the metadata of all installed packages during the concrete update, add method which creates the application/src folder

* remove initial update script


Former-commit-id: 58dd79b
Former-commit-id: 3cd04c72cb64bbfb5ef992f287cb7542426bcd40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants